fix(highlight): keep separator state when reverse-highlight siblings disagree#7079
Merged
Conversation
…disagree
`getHighlightFromSiblings` defaulted neighbor highlight state with `|| true`:
const isNextHighlighted = parts[i + 1]?.isHighlighted || true;
const isPreviousHighlighted = parts[i - 1]?.isHighlighted || true;
`X || true` is always `true`, even when the neighbor exists and is genuinely
`false`. So the `isPreviousHighlighted === isNextHighlighted` guard is always
satisfied and every separator part is treated as inheriting its siblings'
highlight state — even when the siblings disagree. For a value where a
separator sits between a highlighted and a non-highlighted segment, the
reverse-highlight / reverse-snippet output is wrong.
The `?.` optional chaining shows the intent was "default only when the neighbor
is missing". Use `??` so the `true` default applies only when the neighbor is
absent (`undefined`), preserving a real `false`.
Mirrors algolia/autocomplete#1348, which fixes the same bug in the
`autocomplete-preset-algolia` port of this sibling strategy.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
Haroenv
requested review from
a team,
FabienMotte,
Copilot and
shaejaz
and removed request for
a team
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect reverse-highlight / reverse-snippet behavior where separator parts could incorrectly inherit highlight state due to using || true (which collapses real false to true) when computing sibling highlight state.
Changes:
- Replace
|| truewith?? trueingetHighlightFromSiblingsso the default applies only when a sibling is missing. - Add a regression test ensuring separators keep their own state when adjacent siblings disagree.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/instantsearch.js/src/lib/utils/getHighlightFromSiblings.ts | Uses nullish coalescing to correctly preserve false sibling highlight states. |
| packages/instantsearch.js/src/lib/utils/tests/getHighlightFromSiblings-test.ts | Adds regression coverage for the “siblings disagree around a separator” case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
More templates
algoliasearch-helper
instantsearch-ui-components
instantsearch.css
instantsearch.js
react-instantsearch
react-instantsearch-core
react-instantsearch-nextjs
react-instantsearch-router-nextjs
vue-instantsearch
commit: |
FabienMotte
changed the title
… sibling logic
The "multiple matches" case asserted the buggy output: a separator between two
non-highlighted parts ('Fi' and 'Black') was reversed to non-highlighted,
leaving a gap in an otherwise contiguous reverse-highlighted run. With the
`?? true` fix the separator correctly inherits its (agreeing) siblings, so the
reversed "Fi - Black" run is now uniformly highlighted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Haroenv
changed the title
FabienMotte
approved these changes
Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's broken
getHighlightFromSiblings(used byreverseHighlightedPartsfor reverse-highlight / reverse-snippet) computes neighbor highlight state with||:X || trueis alwaystrue, even when the neighbor exists and is genuinelyfalse. So theisPreviousHighlighted === isNextHighlightedguard is always satisfied, and every separator part is treated as inheriting its siblings' highlight state — even when the siblings disagree. For a value where a separator sits between a highlighted and a non-highlighted segment, the reverse-highlight / reverse-snippet output is wrong.Why it happens
||was used where a "default only when the neighbor is missing" was intended — the?.optional chaining shows the intent.false || truecollapses a realfalseneighbor totrue.Fix
Use
??so thetruedefault applies only when the neighbor is absent (undefined). This changes behavior only in the present-and-falsecase (the bug); missing-sibling behavior is preserved byte-for-byte, so no currently-correct case regresses.??is already used throughoutinstantsearch.jssource, so the toolchain supports it.Test
Added a regression case: a separator between a highlighted and a non-highlighted segment keeps its own state. Fails before, passes after.
Context
Mirrors algolia/autocomplete#1348, which fixes the same bug in the
autocomplete-preset-algoliaport of this sibling strategy. The autocomplete code was a faithful port of this function — including the|| true— so the bug exists in both repos.🤖 Generated with Claude Code
Test update
Updated the existing
reverseHighlightedParts"multiple matches" expectation: it asserted the buggy output (a separator between two non-highlighted parts reversed to non-highlighted, breaking an otherwise contiguous reverse-highlighted run). With the fix the separator correctly inherits its agreeing siblings, so the reversed "Fi - Black" run is uniformly highlighted. Note: the equivalent autocomplete repo has no test covering this "separator between two present, agreeing neighbors" path, which is why algolia/autocomplete#1348 went green without a test change.