skills/civitai__civitai
1
0
Fork 0
mirror of https://github.com/civitai/civitai.git synced 2026-09-20 22:08:18 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(search): rebuild the dropdown search provider when its index changes, and carry the typed text across (#4953)

Browse Source 
* fix(search): drop a dropdown search whose filters target the previous index

The header autocomplete and the quick-search dropdown render
`<InstantSearch indexName={...}>` with no `key={indexName}`.
react-instantsearch-core calls `helper.setIndex(indexName).search()` in its
RENDER body, and `<InstantSearch>` renders before its children, so a target
switch fires a search while the helper still carries the previous target's
`<Configure filters>`. The models filter set then lands on another index and
the search backend answers 400 `invalid_search_filter`, which the resilient
client swallows into an empty dropdown. Measured in production RUM: hundreds
of such rejections a day, dominated by models-only attributes arriving at the
images index and by `poi` arriving at indexes no code path ever intends it for.

`SearchLayout` fixes this with `key={indexName}` and says so in a comment. The
dropdowns cannot copy that: remounting clears the query the user is typing.

So the request is rejected in the client instead. `withSearchFilterGuard`
validates each request's filter, facet and numeric-filter attributes against
what its target index declares in `src/server/search-index/filterable-attributes.ts`
and resolves a doomed request to the ordinary empty-result shape without
sending it. Valid requests in the same batch still go to the backend and keep
their position in the response. No UX change: a rejected request already
rendered empty.

It is not silent. A rejection still pushes a Faro RUM error, under its own type
(`SearchFilterAttributeError`) so a locally-rejected request stays tellable
apart from a backend-rejected one (`MeiliSearchQueryError`). Note that a
population which used to beacon under the old type now beacons under the new
one, since it no longer reaches the backend at all.

The guard covers exactly the leaks that name an attribute the new index cannot
filter on — the ones that produce a 400. When the previous target's attributes
are all declared on the new index the stale request is valid there and is sent,
missing whatever clauses that target never built; that direction returns 200,
appears in no error signal, and no attribute check can see it. The module
documents this rather than reading as though it closes the class.

Wiring: the three browser search clients were three hand-rolled compositions
with the same 18-line empty-query short-circuit copied into two of them and
absent from the third. They now come from one `createSearchClient` factory, and
each is exported from its own module so the wiring is reachable from a node
test. That is load-bearing for the tests, not tidying — while the clients were
built inside the `.tsx` files the only possible check was a grep of the
component source, and a source check cannot tell a guarded client that is USED
from one that is merely constructed.

Tests: every shipped client is exercised through its own export, asserting the
negative — the doomed request must not be SENT — plus a positive control that a
filter set built for the index it targets still is. With the guard bypassed at
the factory, those three tests fail on that assertion. A derived ledger
enumerates every module constructing a search client and requires each to be
either the guarded factory or an exclusion whose stated reason is asserted, so
it fails when the population grows and when an exclusion stops holding.

* fix(search): rebuild the dropdown search provider when its index changes

`SearchLayout` keys its `<InstantSearch>` on the index name and says why:
"Needs re-render. Otherwise the prev. index will screw up the app." The header
autocomplete and the quick-search dropdown never got that key, so they carry the
defect the comment describes.

react-instantsearch-core calls `helper.setIndex(indexName).search()` in its
RENDER body (`lib/useInstantSearchApi.js`), and the provider renders before its
children. So on a target switch the search fires against the NEW index while the
helper still holds the PREVIOUS target's `Configure` parameters — the children
that own `filters` have not re-rendered yet. Keying the provider makes React
build a fresh one instead, and the children mount their parameters onto it
before it searches.

This is additive to the request-level guard already on this branch, and it
covers a direction that guard structurally cannot. The guard drops a request
naming an attribute its target index does not declare. When the previous
target's attributes are all declared on the new index — `articles` and
`collections` are subsets of `models` — the stale parameter set is perfectly
valid there, so it is sent, and the clauses the new target builds only for
itself are simply missing from it. That answers 200 and appears in no error
signal. The key closes it at the source: there is no stale parameter set to
send.

The reason the dropdowns could not just copy `SearchLayout` is that remounting
clears the text the user is typing, which lives inside the provider's subtree.
So each root now holds that text in a ref ABOVE the keyed boundary and the
remounted input is seeded from it (`useCarriedSearchText`). Seeding is not only
cosmetic: a rebuilt helper reports an empty query, so the seeded text differs
from it, and that difference is what makes each component's existing "push the
text into the helper" effect fire again — the search is RE-RUN on the new index
rather than the input merely re-displaying the old text. An empty carrier falls
back to the helper's own query, which is what a first mount did before.

Every write to that text goes through the setter the hook returns, so the
carrier can never hold a value the input no longer shows.

Tests, in `src/components/Search/__tests__/dropdown-index-remount.test.ts`:

- A ledger of every `<InstantSearch>` root in `src/`, derived from the tree so it
  fails when the population grows or shrinks. Each keyed root must key on the
  very expression it passes as `indexName` — not merely carry some key, which
  could disagree with the index. `CollectionSelectModal` is the one exclusion and
  its stated reason is asserted, not taken on trust: its index is a fixed member
  of `searchIndexMap`, so there is no switch to survive.
- The carry, asserted structurally on both dropdown roots: the ref is declared
  above the provider, threaded into the content component, and read through the
  hook — and the `useState(query)` shape it replaced, which comes back empty on
  a remount, is banned.
- The hook's behaviour, exercised in the node tier against a real React remount:
  text typed before a key change survives it, and comes back differing from the
  rebuilt helper's empty query. The negative control is the same tree seeded the
  old way, which loses the text — without it, a harness that silently never
  remounted would pass every other assertion while asserting nothing.

Matrix: the four structural tests are RED at `bbeffcf71e` (no `key` prop, no
carrier) and green at HEAD. The behavioural and ledger tests are new-behaviour
guards, not regression tests, and are green at both. Three mutants of the hook
were each killed by their own assertion, with the source restored by digest
after each and a green re-run after the sweep.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(search): keep the chosen category when the search provider is rebuilt

Review follow-up to 19adaeb56e. That commit keyed both dropdown
`<InstantSearch>` roots on their index, which is what stops a search firing
with the previous index's parameters — and the remount it introduces reached
two pieces of state nobody had to think about before, because nothing in these
trees had ever unmounted.

1. The header category selector stopped working, on every page.

   `AutocompleteSearchContentInner` carried

       useEffect(() => {
         if (indexNameProp !== searchTarget) onTargetChange(searchTarget);
       }, [searchTarget]);

   where `searchTarget` comes from `usePathname()`, not from the pick. With no
   remount that effect ran only on navigation. Once the provider is keyed,
   choosing a category remounts the subtree — and a mount runs every effect
   regardless of its dependency array — so the effect read the URL's section,
   found it different from the pick, and put the target back. Measured against
   a real react-instantsearch tree: user on /models picking Images settled on
   `models`, via two mounts and two searches, one of them against an index the
   user was immediately bounced off. `searchTarget` is `models` on every page
   whose first path segment is not a search target, so this was not an edge.

   The sync now lives in `AutocompleteSearch`, above the keyed boundary, for
   the same reason the typed-text carrier does: it has to observe navigation
   without being restarted by a target switch.

2. Both target selectors were uncontrolled, so their displayed label was state
   inside the remounting subtree. A switch reset the label to the default while
   the search really had moved — a control that lies about what it is
   searching. Both now read the target they are searching.

   `QuickSearchDropdown` also defaulted its target to `models` while its
   selector offered `supportedIndexes`, so a caller passing `['users']` and no
   `startingIndex` showed "Users" over a models search. Invisible while the
   selector held its own value; an empty selector once it is controlled. The
   default is now the first supported index, which makes the two agree at the
   source.

3. `AutocompleteSearch`'s refine effect returns early on `searchErrorState`,
   which reads a module-level store and therefore SURVIVES the remount. It was
   not in the effect's dependencies, so a tree that remounted while search was
   unavailable restored the typed text, returned early, and never refined when
   the flag cleared — a populated input over an empty helper query until the
   next keystroke. Added to the dependencies.

   Both components' refine decisions now go through one `shouldRefineSearchQuery`
   predicate rather than two hand-written conditions that had already drifted
   apart.

Also: the guard test added earlier on this branch stated, as the reason the
guard exists, that the dropdown roots "cannot" be keyed because remounting
would clear the typed query. This branch falsifies that, and its exclusion rule
("has no key={indexName}") no longer discriminates now that every root is keyed.
Comments only — no assertion changed.

Tests: 22 in `dropdown-index-remount.test.ts`, 7 of them RED with the two
components at 19adaeb56e's parent and green at HEAD. The harness now models the
helper as per-mount state and drives the shipped predicate, so the claim that a
remount RE-RUNS the search is observed (a second refine with the carried text)
rather than asserted by a test name. Added: a negative control that the carrier
is per-instance rather than module-scope, and the blocked/recovered pair.

Mutation sweep, source restored by digest after each and a green re-run after
the batch: 10 mutants, 10 killed, each by an assertion naming its own behaviour.
Two of them — pinning either root's index to a constant — SURVIVED the first
round, because the ledger checked the expression in the tag while both roots
hoist it into a local; the check now follows a bare identifier to its
declaration, and both then die. That pair is the reason this commit's ledger is
worth more than the previous one's.

Not verified: the browser tier does not run on the authoring host, so the real
click path is still unexercised. Finding 1 was reproduced by a subagent against
a real react-instantsearch tree in a scratch harness, not by a test in this
repo; what ships here for it is a structural guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(search): keep the target selector inside the set it offers

Round-2 review of cf5c69e235. Making both target selectors controlled closed a
label desync and opened a narrower one, and the deselect path the controlled
value exposed turns out to reach a payout field.

1. A controlled `<Select>` whose value is not among its options keeps showing
   the PREVIOUS option's label. Mantine's value→label sync runs only when the
   value resolves to an option (`Select.mjs`: the `[value, selectedOption]`
   effect takes neither branch otherwise) and the uncontrolled fallback that
   used to clear it is skipped once `value` is passed. The header search reaches
   that state without any user action: its target follows the URL section, and a
   section whose feature flag is off is filtered out of the options — so on
   `/images` with image search disabled the selector read "Models" while the
   provider searched the images index. Uncontrolled it read blank. Both
   selectors now pass `null` when the target is not an offered option, which is
   what blanks it; the offered set is hoisted so the value is clamped to the
   same list the options are built from.

2. `QuickSearchDropdown`'s selector did not set `allowDeselect`, and Mantine
   defaults it to `true` — so a single-option selector is deselectable, and the
   deselect hands `null` to the change handler, which fell back to `models`.
   Callers read the picked entity as the type their `supportedIndexes` names;
   `CosmeticShopItemUpsertForm` passes `['users']` with the selector visible and
   writes the picked id into `meta.paidToUserIds`, which
   `cosmetic-shop.service.ts` splits a price across. `allowDeselect={false}`,
   and the fallback now goes to the first supported index rather than to
   `models`. Pre-existing on both counts; the previous commit fixed the
   initial-state half of the same inconsistency and left the runtime half.

3. Corrections to claims, no behaviour change. The `supportedIndexes` fallback
   comment described an observed defect; enumerating all ten call sites shows
   every one either passes `startingIndex` or supports `models` first, so it is
   a forward guard and now says so. `shouldRefineSearchQuery`'s docblock
   required every input to appear in the calling effect's dependency array,
   which `selectedItem` does not — the rule is real only for a source that
   OUTLIVES the remount, and the sentence now says that and names both cases.

Tests: 22, unchanged in count, 7 still RED with the two components at
bbeffcf71e. Five mutants that survived round 2 now die:

  - the URL-follow sync left in place AND re-added inside the subtree (the
    consolidation that forgets to delete one copy — this restores the reverted
    category pick with every other assertion satisfied)
  - its dependency array emptied, so it stops following navigation
  - `searchErrorState` dropped from the predicate ARGUMENT while left in the
    dependency array, which the dep-array check alone cannot see
  - the selector value unclamped
  - `allowDeselect` removed

Two spellings named in review are accepted as still walkable: an index written
`searchIndexMap['models']` or as an imported constant satisfies the
constant-index check, because a source-text guard cannot follow either. The
comment says what these guards pin — spelling and file order — rather than
implying tree position.

Still not verified: the browser tier does not run on the authoring host, so the
real click path remains unexercised end to end, and finding 1's stale-label
behaviour was established from Mantine's source rather than by rendering it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): make the selector guards constrain the thing that can go wrong

Round-3 review of a4ca0d1ff9. The safety lane came back clean; the test lane
found that the two production behaviours added that round — the value clamp and
the `enabledTargets` hoist — shipped with assertions that did not pin the part
that can actually break. Test-only, plus one type error.

  - The clamp assertion was RECEIVER-AGNOSTIC: it matched
    `… value === indexNameProp) ? indexNameProp : null` whatever `.some()` was
    called on. Clamping against the UNFILTERED list restores the exact stale-
    label defect the clamp exists to prevent, and every test stayed green. Now
    pinned to `enabledTargets.some(…)`.
  - Nothing asserted `data={enabledTargets}`, so the offered list and the list
    the value is clamped against could diverge — the one invariant the hoist's
    own comment states. Now asserted, in both files.
  - `allowDeselect={false}` was pinned on `QuickSearchDropdown` only.
    `AutocompleteSearch` is the copy where it matters more: its change handler
    casts the `null` a deselect produces straight through with no fallback, so
    `searchIndexMap[null]` reaches the provider as an undefined index. Pinning
    only the sibling makes the natural "these two selectors duplicate props"
    tidy-up delete the unguarded one. Now asserted on both.
  - The "exactly one writer" count and the position check read RAW source, so a
    comment naming `setTargetIndex(searchTarget)` — including one written to
    warn against the doubled-writer mutation the count exists to catch — turned
    the test red. Same prose-satisfies-a-token hazard the refine-deps check was
    fixed for one round earlier, running in the mirror direction. All the
    counting and locating checks now strip comments first, through one shared
    helper.
  - The follows-nav regex required the dependency array to be EXACTLY
    `[searchTarget]` and the write to be the last statement in the effect. A
    legitimate added dependency, or the `eslint-disable-next-line` line this
    same file already uses twice, went red for no defect. It now requires the
    array to CONTAIN `searchTarget`.
  - `ReturnType<typeof readdirSync<{ withFileTypes: true }>>` is not valid
    TypeScript — `readdirSync` is not generic in this `@types/node`. Nothing
    caught it: `tsconfig.json` excludes `src/**/__tests__/**`, so the repo
    typecheck cannot see it, and esbuild strips types for vitest. `Dirent[]`.

Sweep: 5 new mutants, 5 killed — the clamp pointed at the unfiltered list in
either component, `data` reverted to the unfiltered list, `allowDeselect`
removed, and `allowDeselect` "satisfied" by commenting it out. Plus two
FALSE-POSITIVE controls that must stay green and do: a comment naming
`setTargetIndex(searchTarget)`, and a legitimate extra dependency with an
eslint-disable line above the array.

Matrix unchanged: 22 at HEAD, 7 red with both components at bbeffcf71e.
`tsc -p tsconfig.tests.json` is now clean for these files as well as the repo
typecheck, which does not cover them.

Accepted and left open, with the reason in the comments rather than a guard:
an index written `searchIndexMap['models']` or as an imported constant still
satisfies the constant-index check, because no source-text guard can follow
either binding. The full-call-expression and clamp assertions are pinned to
spelling, so a rename or a prettier wrap reddens them for a non-defect; both
sit near the 100-column limit today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(search): restore the two components a3e8d80062 reverted by accident

a3e8d80062 was meant to be test-only. It also rolled `AutocompleteSearch.tsx`
and `QuickSearchDropdown.tsx` back to bbeffcf71e — no `key` on either
`<InstantSearch>`, no typed-text carrier, no selector clamp — while leaving the
tests that assert all of it in place.

Cause, recorded because it is not obvious and it is a trap for anyone measuring
a red/green matrix the way this branch has been: `git checkout <ref> -- <path>`
writes the INDEX as well as the working tree. The matrix run checked the two
components out at the base, and the restore afterwards copied the files back
into the working tree only. The index still held the base blobs, `git commit`
commits the index, and `git add` of the two test files did not touch them. The
`MM` in `git status` was the tell and it was not read.

This restores both files to exactly their a4ca0d1ff9 content — verified by
`git diff a4ca0d1ff9 -- <both paths>` being empty, and byte-identical to the
copies the round-3 mutation sweep was run against.

No behaviour is intended to change from a4ca0d1ff9. Every test that a3e8d80062
added still passes; the seven that go red with the components at the base are
red for that state and green again here, which is what makes the accident
visible rather than silent.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): pin the operators, not just the calls

Round-4 review. The round-3 tightenings were REPLACEMENTS rather than additions,
and each one dropped the half of the expression that says what is DONE with the
answer. Three one-character or one-clause mutants killed the feature outright
with the whole suite green.

  - The selector clamp was asserted as its predicate only, so negating the
    condition — show the target only when it is NOT an offered option — passed
    everything, as did replacing the `null` branch with a first-option fallback,
    which reinstates exactly the wrong-label lie the clamp exists to prevent.
    The whole `value={…}` expression is now pinned, in one shared constant so
    the two components cannot drift apart.
  - The refine gate was asserted as the call, never the leading `!`. Dropping it
    inverts both effects — they return early exactly when they should refine —
    which is Part 2's entire mechanism, dead, with nothing red. Nothing else in
    the repo sees it either: the browser specs stub `useSearchBox` or mock the
    component wholesale. Both `if (…)` statements are now pinned in full.
  - The carrier was asserted as `useCarriedSearchText(carriedSearchText, query)`,
    which a call whose RESULT is discarded still satisfies — call it, then seed
    from `useState(query)` beside it, and the typed text is per-mount state again.
    The destructuring is now part of the assertion.

Sweep: 6 mutants, 6 killed, each on its own assertion — clamp negated, clamp
given a first-option fallback, refine negation dropped in either component,
carrier result discarded in either component.

Also: the `stripComments` helper now runs on every check that counts or locates
a token, including the two it had been left off (the carrier position check and
the ledger's membership test); the follows-nav window can no longer skip over an
intervening bracket pair to find a different dependency array.

Two things are now said out loud rather than left implied. `allowDeselect={false}`
on `AutocompleteSearch` predates this PR — it is an INVARIANT GUARD, not
regression coverage, and it is labelled as one. And `QuickSearchDropdown`'s
`supportedIndexes`-aware fallback has no test, deliberately: no caller reaches it,
so a test for it would also be an invariant guard.

Matrix: 23 at HEAD, 8 red with both components at bbeffcf71e (the new refine-gate
test is the eighth).

🔴 The base measurement used `git show <ref>:<path> > <path>`, which writes the
working tree ONLY. `git checkout <ref> -- <path>` writes the INDEX as well, which
is how the previous round's measurement got committed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): pin the wiring, and state the tracking rule as a shape

Round-5 review. Three gaps, all in the same direction: the guards pinned what
is DECLARED and never what is USED.

  - The carrier assertion pinned the declaration, so `value={search}` on the
    input could become `value={query}` in either dropdown and the whole suite
    stayed green — after a remount the input renders the fresh helper's empty
    query while the carrier holds the typed text, which is the defect this PR
    exists to fix. The input wiring is now pinned on both sides.
  - The "index must track something" check enumerated constant SPELLINGS to
    reject. It caught `searchIndexMap.models` and a string literal and let
    `searchIndexMap['models']` and an imported `IMAGES_SEARCH_INDEX` through —
    both one-token edits, both leaving `key` and `indexName` in agreement while
    the selector stops switching index at all. It now states what a tracking
    index IS: a `searchIndexMap[targetIndex]` subscript, asserted per dropdown.

    🔴 The generic identifier resolver that check used is DELETED rather than
    widened. Scoped to the two dropdowns it is correct; applied generically it
    bound `SearchLayout`'s `indexName` — a PROP — to an unrelated
    `const indexName = Object.keys(uiState)?.[0]` elsewhere in that file, and
    went red on a root that has nothing wrong with it. Caught by running it.
  - `AutocompleteSearch`'s refine gate is wrapped across two lines by prettier,
    so pinning the `if (…)` alone left its `return` unpinned; neutering the
    consequent refined during an outage with the gate correctly spelled. Matched
    across the break now.

Also: the availability test no longer restates what the refine-gate test already
subsumes — one change should redden one test — and the clamp constant carries
back the brittleness caveat it lost, now with the measurement: the pinned line is
EXACTLY 100 characters in both files against `printWidth: 100`, so one more level
of indentation reddens it on formatting alone.

One new test, labelled INVARIANT because it pins a case production cannot reach:
carried text outranking a NON-EMPTY helper query. Both non-empty at once cannot
happen — a rebuilt helper always reports `''` — which is exactly why it is the
only thing that can see the hook's own `seedCarriedSearchText` call with its
arguments swapped. That mutant survived every other test in the file.

Sweep: 8 mutants, 8 killed, each on its own assertion — the input reading the
helper query in either dropdown, the index pinned to an imported constant, to a
bracket literal and to a dot constant, the refine consequent neutered, and two
against the hook itself (the setter not writing the carrier; the seed arguments
swapped). The previous rounds' sweeps had no hook mutants at all, so 12 of the
tests had never been watched go red for a defect in the code they cover.

Matrix: 24 at HEAD, 8 red with both components at bbeffcf71e. The base half says
nothing about the 12 hook-tier tests — the hook does not exist at that commit, so
it is held at HEAD for the measurement; the sweep above is what covers those.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): pin the category pick, and put back what SearchLayout lost

Round-6 review. Two findings, both introduced by earlier rounds of this same
ladder, both test-side.

1. Nothing asserted that PICKING a category reaches the state the index is
   derived from. The suite pinned the selector's value, its options and its
   deselect behaviour, and the input's `value={search}` / `setSearch(value)` —
   and left the handler between them unpinned. Five mutants were green:
   neutering either selector's `onChange`, emptying either `handleTargetChange`,
   and neutering the `onTargetChange`/`onIndexNameChange` prop. Manual category
   switching is dead in all five, and `AutocompleteSearch` still LOOKS alive
   because its URL-follow effect keeps calling `setTargetIndex` — only the
   user's own pick stops working. The exactly-one-writer count cannot see it:
   `onTargetChange(v as TKey)` is not that pattern.

   This is the same hole as the input wiring one round earlier, one level up.
   Both handler hops are pinned now, in a test named for them.

2. Deleting the generic identifier resolver last round left `SearchLayout`
   UNGUARDED on its index expression. It was the only root that check reached
   which the per-dropdown replacement does not, so pinning both its `key` and
   its `indexName` to the same constant went from red to green — two identical
   constants satisfy `key === indexName`, and every search page would then
   search one index. Closed by asserting the prop expression is not itself a
   constant, which is prop-scoped and therefore cannot re-create the false
   positive the resolver had (it bound `SearchLayout`'s `indexName` to an
   unrelated `const indexName = Object.keys(uiState)?.[0]`).

   Same check also closes the dropdown variant the resolver used to catch:
   pinning the JSX to a constant while leaving the tracking `const` in place.

Also: `INDEX_TRACKS_TARGET` moved out of the carrier test into its own, so that
mutation class no longer reports under a title about carrying typed text, and
its `=` now tolerates a prettier wrap the way the clamp constant documents.

Sweep: 7 mutants, 7 killed, each on its own assertion — the `SearchLayout`
constant pin, the QuickSearchDropdown JSX constant pin, and all five selector
wiring mutants.

Matrix: 26 at HEAD, 10 red with both components at bbeffcf71e.

NOT closed, and named rather than left silent: `stripComments` itself has no
killing mutation — deleting either half leaves the suite green, because only a
composite (comment-out the code AND disable the matching strip) can show it.
A test for it would be a test of the test file, breakable only by an edit to
that same file, so it is declined rather than forgotten.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): close the middle hop, and stop rejecting correct code

Round-7 review. Two findings, and a false positive the fix for the second one
exposed.

1. `AutocompleteSearch`'s `handleTargetChange` body was unpinned. The test added
   last round pins both ends of the chain on that component — the selector's
   `onChange`, and the `onTargetChange` prop — and stopped. Deleting
   `setTargetIndex(value)` between them left all 26 green while picking a
   category in the header search never writes the target: the provider never
   re-keys and the controlled selector snaps its label back. The sibling was
   covered only because `setTargetIndex(value ?? fallbackIndex)` happened to be
   on the list for a different reason, which is what made the asymmetry hard to
   see. Pinned now, and the test says which of its assertions are invariant
   guards — four of the five spellings are unchanged from the PR base, so its
   red-at-base comes from the fallback line alone, not from the claim in its
   title.

2. The `SearchLayout` guard added last round enumerated constant SPELLINGS to
   reject — the exact hole the file's own docblock warns about two paragraphs
   earlier. An imported `MODELS_SEARCH_INDEX` walked through it, and every
   search page would then search one index. Restated positively: that root's
   index must be the parameter the component destructures.

   🔴 It also REJECTED CORRECT CODE. Writing the tracking expression inline on
   the provider — the shape `AutocompleteSearch` had at the PR base, plus a key
   — failed it. A guard that fails on correct code as well as passing broken
   code is worse than none, and only the false-positive control in the sweep
   caught it; every mutant in that batch behaved.

   Removing it re-opened a mutant it had been covering by accident: pinning a
   dropdown's JSX to a constant while leaving an unused tracking `const` above
   it. So the derive check now follows the expression the PROVIDER receives and
   resolves it one hop when it is a hoisted identifier. Both halves are needed
   and neither is sufficient — declaration-only lets the JSX be pinned,
   JSX-only rejects the inline form.

Sweep: 8 mutants — `handleTargetChange` emptied; `SearchLayout`'s index pinned
to an imported constant, to a map member and to a string literal; a dropdown's
JSX pinned while its const stays; either dropdown's const pinned — all 8 killed
on their own assertion, plus a FALSE-POSITIVE CONTROL that must stay green and
now does: the tracking expression written inline with no hoisted local.

Matrix: 26 at HEAD, 9 red with both components at bbeffcf71e — one fewer than
last round, because teaching the derive check to accept the inline form makes it
GREEN at the base, correctly: both roots already derived their index from the
target there. What the base lacked was the `key`. That test is relabelled an
invariant guard rather than counted as regression coverage.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): a comment was enough to defeat the SearchLayout guard

Round-8 review, on the guard round 7 added.

  - It read UN-STRIPPED source — the only token-locating check in the file that
    did not go through `stripComments`, which is the exact failure that helper's
    own docblock describes. The mutant it exists for survived behind an ordinary
    code comment: remove the index prop, pin the provider to an imported
    constant, leave a line in the parameter list saying why. The identical pin
    WITHOUT the comment was red, so the comment, not the pin, was deciding the
    verdict. That pair is the measurement; one of them alone proves nothing.
  - It also rejected correct code in two shapes — normalising the prop through
    one local (`const resolvedIndex = indexName ?? …`), and the `export const`
    component style both sibling roots in this ledger already use — because it
    required the provider's literal expression to appear inside a literal
    `export function SearchLayout({…})`.

    Restated as what is actually required: the expression must REFERENCE the
    prop. Resolved one hop only when it does not already do so — this file also
    holds an unrelated `const indexName = Object.keys(uiState)?.[0]`, and
    resolving unconditionally binds to that and reddens a healthy root. That is
    the same trap a generic resolver hit two rounds ago; it is avoided here by
    not resolving what needs no resolving.

The one-hop resolution is now one shared helper for both the dropdown and the
`SearchLayout` check, and tolerates a type annotation on the declaration —
`const indexName: SearchIndex = searchIndexMap[targetIndex]` used to be rejected.

Also softened a coverage claim rather than widening a check: QuickSearchDropdown's
middle hop is a free-floating substring, weaker than AutocompleteSearch's pinned
declaration, and the comment now says so instead of implying parity. Tightening
it symmetrically would reject a correct consolidation into a shared handler.

Sweep: 2 mutants killed (the comment-shielded constant pin and its no-comment
control), and THREE FALSE-POSITIVE CONTROLS that must stay green and do — the
prop normalised one hop, a type-annotated declaration, and the tracking
expression written inline. This ladder has now produced one guard that passed
broken code and two that failed correct code; the controls are the half that
catches the second kind.

Matrix unchanged: 26 at HEAD, 9 red with both components at bbeffcf71e.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): the SearchLayout guard's live branch was a tautology

Round-9 review, on the fix round 8 landed. It closed the comment-shield hole and
opened a worse one in the same four lines.

The guard short-circuited the one-hop resolution whenever the expression already
contained the token `indexName` — which the live code's expression IS. So the
live branch compared the expression against a pattern it had just been tested
against: it could not fail, and the assertion that guards this root never
evaluated anything. Measured: dropping `indexName` from the destructuring,
leaving it in the prop type so no caller breaks, and shadowing it with a local
`const indexName: SearchIndex = 'models_v9'` left all 26 green — exactly the
defect the docblock above it names. The same mutant was RED against the
pre-round-8 file, so this was a regression that fix introduced, not a pre-existing
gap.

It also rejected the alias idiom both sibling roots in this ledger already use
(`indexName: indexNameProp`), because `\bindexName\b` does not match
`indexNameProp` and a destructuring alias is not a `const` declaration.

Restated as the property that is actually required: the index expression, after
one hop, must reference a name the component BINDS in its parameter
destructuring. Resolution is now scoped to the component rather than
short-circuited — slicing from the declaration is what makes it safe, since the
unrelated `const indexName = Object.keys(uiState)?.[0]` sits above the component
and is out of scope. That removes the tautology and the alias rejection at once.

Sweep: 2 mutants killed — the shadowed local constant, and a constant pin behind
a comment — with 2 FALSE-POSITIVE CONTROLS that stay green: the alias idiom, and
the prop normalised through one local.

Also swept the last unstripped read (the `static-index` row) into `stripComments`
for consistency, and recorded in the docblock that the anchored tracking pattern
rejects a `useMemo`-wrapped declaration — correct code, declined rather than
accommodated, with the reason.

Matrix unchanged: 26 at HEAD, 9 red with both components at bbeffcf71e.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): delete the SearchLayout index guard instead of tightening it again

Round-10 review, and the end of this ladder. The recommendation was to take the
guard down rather than fix it a fifth time, and it is right.

Four rounds were spent on one `if` block, each round's fix producing the next
round's finding. It passed broken code twice — an imported constant, then a
shadowing local behind a comment — and rejected correct code three times: a
normalisation hop, the `export const` component style both sibling roots already
use, and a reordered destructuring. Round 9's version also LOST a kill round 8
had while adding brittleness, and its failure message misdiagnosed every one of
those refactors, printing "it is pinned" about code that was not. It was a regex
approximation of scope resolution and name binding, and it never reached a fixed
point.

The requirement does not survive being questioned. `SearchLayout.tsx` is not
touched by this change — it is not among the five files in the diff — so the
guard protected an invariant no commit here can violate, at the cost of
reddening ordinary refactors. It contributed nothing to the red-at-base matrix,
then or now.

What remains for that root is `key === indexName`, which is order-independent,
style-independent, alias-tolerant, and is the claim the ledger exists to make.
A comment in its place records what was tried and asks for a defect before
anyone adds it back.

Verified the deletion removed no coverage that matters: the four dropdown
mutants still die — either root's tracking `const` pinned to a constant, a
root's JSX pinned while the `const` stays, and a `key` removed. Matrix
unchanged, 26 at HEAD and 9 red with both components at bbeffcf71e, because the
deleted assertion was never among those 9.

Also removed the now-dead comment strip inside `openingTag` — every caller
passes `stripComments`ed source — and corrected its docstring, which still
advertised it.

Net −45 lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(search): drop the client-side attribute guard, keep the keyed roots

The guard had no reachable true positive left. It is applied at exactly one
site, that factory has exactly three consumers, those reach exactly two
components, and both now carry key={indexName} on their <InstantSearch> root.
react-instantsearch-core only runs setIndex(indexName).search() in its render
body when prevProps.indexName !== props.indexName, and prevPropsRef is seeded
with the current props, so that branch cannot fire on the fresh mount a key
produces. There is no stale parameter set for the guard to catch.

What it still did there was save a round trip its own docblock called "no UX
change", relabel a beacon this PR expected to fall to ~0, and carry a
live-superset-of-code drift mode that can only fire falsely and would silently
empty a working search. Dropping it also leaves the comics_v1 beacons on the
existing MeiliSearchQueryError type, which is the only signal a separate
index-configuration issue has.

Removed with it, each because its only justification was serving the guard:

  searchFilterGuard.ts, searchFilterGuard.test.ts and
  __tests__/search-client-filter-guard.test.ts;

  the SearchFilterAttributeError beacon type, so the existing
  MeiliSearchQueryError population is left undisturbed;

  stripQuotedMeiliValues in meili-filter.ts -- sole consumer was the guard's
  attribute scanner, and it had no test outside the guard's;

  createErrorReportCap, pushSearchClientError and emptySearchResult in
  resilientSearchClient.ts -- each was extracted so the guard could share it,
  and each docblock says so. With one caller the extraction is unmotivated and
  the docs become false, so the file is restored to its pre-PR content byte for
  byte;

  the search-client-factory consolidation and the autocomplete/quick-search
  client modules. The factory's own docblock gives the guard as its reason: "a
  wrapper added to two of three call sites leaves the third silently
  unprotected". With no wrapper to apply, what is left is de-duplication --
  defensible, but a separate change, and it would land untested once the
  ledger that exercised it goes. AutocompleteSearch.tsx, QuickSearchDropdown.tsx
  and search.client.ts get their pre-PR client construction back verbatim;

  EXCLUDED_CLIENT_SITES, which lived in the deleted ledger. Its surviving
  invariant -- every <InstantSearch> root is keyed or targets a fixed index --
  is covered by INSTANT_SEARCH_ROOTS in dropdown-index-remount.test.ts, which
  is derived from the tree rather than hand-listed.

allowDeselect={false} on QuickSearchDropdown's target selector also leaves this
PR. It is a one-line change to a control whose only reachable call site writes
into a funds-distribution field, so it is worth its own review rather than a
line in a 13-file diff. Its assertion leaves dropdown-index-remount.test.ts
with it; the AutocompleteSearch one stays, because that prop predates this PR.

Removing it makes QuickSearchDropdown's fallbackIndex reachable -- a deselect
hands the change handler null -- so the comment that called it unreachable is
corrected rather than left standing.

The false claim that motivated the guard is gone with it. The sentence
"the dropdown surfaces cannot use that remedy because remounting clears the
user's typed query" is untrue at HEAD: this PR does exactly that, twice, and
useCarriedSearchText is what carries the text across. Swept the whole tree for
it with comment leaders and newlines normalised away, since it wrapped across
comment lines; two differently-shaped scans over all 10,925 tracked files, each
with a positive control, found it only in the two files deleted here.

* fix(search): make the carried text survive the blur, and keep the selector out of the remount

Round-1 fix round on #4953. Four approved changes, no behaviour outside them.

1. The carry was INERT on AutocompleteSearch. Its input bound onBlur to the same
   handler as the clear button, and that handler writes the carrier — so clicking
   the category selector (which blurs the input first) emptied the carrier a moment
   before the index switch it exists to survive. useCarriedSearchText now also
   returns a display-only clear; blur uses it, the clear button still goes through
   the setter, and onClear?.() fires from both so the mobile overlay still closes.
   The carrier is emptied when NAVIGATION moves the target, so only a pick from the
   selector re-seeds and abandoned text cannot reappear on the next link followed.

2. Both index selectors are now rendered ABOVE <InstantSearch>. The provider
   returns null until its start effect has run, so a key change commits one render
   with no subtree at all: the control the user just clicked was destroyed and
   rebuilt by their own click, dropping focus to <body>. Neither selector consumes
   the provider's context. Whether that null commit is PAINTED was not measured.

3. The selector-value clamp pin is now compared with whitespace removed from both
   sides. Both call sites had sat at exactly printWidth, so one rename or one indent
   level would have reddened it for a prettier re-wrap. Mutating the clamp still
   fails it, and a deliberate re-wrap does not.

4. QuickSearchDropdown's fallbackIndex comment said the target stays inside the set
   the selector OFFERS. It does not: the clamp reads supportedIndexes, and the
   offered list narrows that further by feature flag. Reworded to what the code does.

Coverage: the blur-then-switch path had none — every carry assertion was a source
spelling check and the behavioural harness modelled a remount with no blur. Added
that case plus a same-carrier negative control that differs only in which setter the
empty value goes through, a file-order pin for each selector's position, and a
spelling pin for the blur wiring including the onClear?.() in both handlers.

* fix(search): bound the carried search text, and correct four claims it outgrew

Round-2 fixes on the carried-search-text change.

BEHAVIOUR (6 added / 2 removed executable lines, all in AutocompleteSearch.tsx)

The carrier was emptied on exactly one path: a navigation that CHANGES
`searchTarget`. That is narrower than the comment claimed, in three ways —
any first path segment outside `targetData` collapses to 'models', so
navigation within a section never fires it; and both `handleSubmit` and
Escape reached the display-only blur clear, which leaves the carrier loaded.
So text typed on '/', blurred away (no clear button remains, since
`clearable` keys on the visible query) and left behind could resurrect on a
later category pick, and be searched.

Submitting and pressing Escape are unambiguous "done with this text" signals,
so both now discard the carrier, through one named function. NOT inside
`blurInput`: both of its callers are those two paths today, so the placement
is behaviourally identical at this head, but `blurInput` is a DOM verb and
the discard is a claim about intent — a future caller that is not a done
signal must not inherit it. There is no imperative blur either way; the
handle exposes `focus` only. And NOT on plain blur: reaching the category
selector requires blurring, which is what made the carry inert before.

The same-section-navigation residue remains, deliberately. Both the effect
and the hook doc now say so instead of implying a tighter bound.

CLAIMS CORRECTED

- The `enabledTargets` comment described the display-value clamp with the
  same word the adjacent `fallbackIndex` comment uses for a different set.
  Rewritten in both dropdowns to say which set each narrows.
- QuickSearchDropdown's Select: the stated reason for CONTROLLED was that the
  remount reset the Select's internal state. The hoist removed that mechanism
  and `targetIndex` there now has exactly one writer, so the justification is
  recorded as GONE rather than replaced. What still needs `value` is the
  `null` branch beside it. The AutocompleteSearch sibling is left alone — it
  genuinely has a second writer.
- "a key change commits one render in which the subtree is gone" understated
  it: `<InstantSearch>` renders null whenever its instance is not started, and
  start runs from a subscription callback after a commit, so every fresh
  provider does this. Scoped in both files and in the test. No paint claim.

SCAFFOLDING

- The `defaultValue` guards discriminated nothing. QuickSearchDropdown's
  needle `defaultValue={enabledTargets[0]}` is an object where Mantine wants
  `string | null` and fails TS2322, so it could never have been written; the
  realistic reverts typecheck and were MEASURED to leave the round-1 suite
  fully green (30/30) on both dropdowns. Now pinned on the PROP, scoped to
  the selector's own source.
- Removed the dead `onTargetChange` alternation arm from the one-writer count
  and the two comments citing that spelling; it exists nowhere in src/, and
  an earlier assertion in the same test already fails on the base spelling.
- "one hop, not the three it used to be" then drew two. Corrected, and the
  removed hop named.
- `containsIgnoringWhitespace`'s "no mutation is reachable by whitespace
  alone" is true of its needle, not of the helper. Scoped to the needle.
- Stated plainly that the three discards are spelling-pinned only, and that
  they are not sufficient.

VERIFICATION

Red-at-base / green-at-HEAD: 1 test, `AutocompleteSearch discards the carried
text on submit and on Escape` — 1 failed / 30 passed at d13739cc, 31 passed
at HEAD. Behavioural, not a rename artifact: it names a function the base
does not have. Every other changed assertion is green at base and is labelled
an invariant guard in place.

Mutation sweep, 6 mutants, all killed by their own guard's message, restore
digest-verified between each, post-sweep 31/31 green.

Gates: typecheck 0 errors (negative control: injecting the old guard's own
needle produced 1 TS2322, then 0 on restore); vitest unit over both component
dirs 224/224 across 9 files; eslint 0 errors / 11 warnings, identical count at
base (negative control: an injected violation moved it to 1); prettier clean
on all four files (negative control: an injected reformat produced one [warn]).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(search): correct three claims the previous commit's own comments got wrong

Self-review of the round-2 comments against the code at this head, which is
the check this ladder keeps skipping. Comments only; no behaviour change.

- "Both of `blurInput`'s callers are these two paths today" was written from
  the pre-change shape. After the change `blurInput` has exactly ONE caller,
  `blurAndDiscardCarriedText`. The argument for not putting the discard inside
  it is unchanged and now stated from what the code actually is.

- "The other two discards are `blurAndDiscardCarriedText` (submit, Escape) and
  the clear button" names three things as two. Split: one explicit discard
  (`blurAndDiscardCarriedText`, on two paths) plus the setter path an emptied
  input takes.

- "that name exists nowhere in `src/` now", of `onTargetChange`, is false:
  the same commit reintroduced it into the test file's own prose as a
  historical reference. Narrowed to what was actually measured — no SOURCE
  file spells it.

Gates re-run at this head: typecheck 0 errors; vitest unit over both
component dirs 224/224 across 9 files; prettier clean on all four; the
6-mutant sweep still kills all six, post-sweep 31/31.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(search): restore the cross-spelling leftover-copy kill, on the argument

F1. The previous round narrowed the exactly-one-writer count from
(?:setTargetIndex|onTargetChange)\(searchTarget to setTargetIndex alone, on the
rationale that the dropped arm "could not have discriminated anyway - a revert to
the base spelling fails the indexOf assertion above before reaching this line".
That sentence reasons about a FULL revert. The guard's own comment three lines up
says it exists for the PARTIAL one: hoisting the sync while leaving the old copy
in place. In that shape the hoisted setTargetIndex(searchTarget) is still there,
so indexOf succeeds and execution does reach the count - and the leftover copy is
spelled onTargetChange(searchTarget as TKey), which is exactly what the merge base
writes. The narrowed count is blind to it. The sentence is deleted, not replaced
with a fresh one.

Restored as a count on the ARGUMENT rather than on the alternation: a leftover copy
carries whatever name the boundary it crossed had, so a two-name alternation goes
blind the moment a third appears. Any call taking searchTarget is counted; today
the sync is the only one. Cost stated in the comment - a legitimate future reader
of searchTarget reddens this too, and the fix is to re-pin, not to loosen.

Measured, whole file, 31 tests, restores digest-verified between arms:
  leftover-copy mutant + pre-fix test      GREEN 31/31   (the coverage loss)
  leftover-copy mutant + this head         RED, this assertion's own message
  same-spelling duplicate + this head      RED           (nothing else narrowed)
  same-spelling duplicate + pre-fix test   RED           (pre-fix instrument works)
  clean tree at this head                  GREEN 31/31   (before and after the sweep)
Each RED was the only failing test in its run.

F2. The blurAndDiscardCarriedText comment called submit and Escape "the two ways a
user says they are finished", which is not the set: handleItemClick navigates and
fires the same onSubmit?.() while discarding nothing. Scoped to the two blur paths
and the pick path named, with what actually decides the carrier there. Comment
only - no executable change in AutocompleteSearch.tsx.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Zachary Lowden
2026-09-19 12:58:19 -05:00
committed by GitHub
parent a75fb42290
commit 854595f7d3
4 changed files with 1384 additions and 342 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/components/AutocompleteSearch/AutocompleteSearch.tsx
+329 -237
View File
@@ -30,6 +30,10 @@ import { instantMeiliSearch } from '@meilisearch/instant-meilisearch';
import { withUserHydration } from '~/components/Search/userHydration';
import { env } from '~/env/client';
import { createResilientSearchClient } from '~/components/Search/resilientSearchClient';
import {
shouldRefineSearchQuery,
useCarriedSearchText,
} from '~/components/Search/useCarriedSearchText';
import { quoteMeiliValue } from '~/components/Search/meili-filter';
import {
autocompleteAvailability,
@@ -89,36 +93,36 @@ type Props = Omit<AutocompleteProps, 'data' | 'onSubmit'> & {
// never reaches `useInstantSearch().status`, so we can't key off that).
const searchClient: InstantSearchProps['searchClient'] = withUserHydration(
createResilientSearchClient(
{
...meilisearch,
search(requests) {
// Prevent making a request if there is no query
// @see https://www.algolia.com/doc/guides/building-search-ui/going-further/conditional-requests/react/#detecting-empty-search-requests
// @see https://github.com/algolia/react-instantsearch/issues/1111#issuecomment-496132977
if (requests.every(({ params }) => !params?.query)) {
return Promise.resolve({
results: requests.map(() => ({
hits: [],
nbHits: 0,
nbPages: 0,
page: 0,
processingTimeMS: 0,
hitsPerPage: 0,
exhaustiveNbHits: false,
query: '',
params: '',
})),
});
}
{
...meilisearch,
search(requests) {
// Prevent making a request if there is no query
// @see https://www.algolia.com/doc/guides/building-search-ui/going-further/conditional-requests/react/#detecting-empty-search-requests
// @see https://github.com/algolia/react-instantsearch/issues/1111#issuecomment-496132977
if (requests.every(({ params }) => !params?.query)) {
return Promise.resolve({
results: requests.map(() => ({
hits: [],
nbHits: 0,
nbPages: 0,
page: 0,
processingTimeMS: 0,
hitsPerPage: 0,
exhaustiveNbHits: false,
query: '',
params: '',
})),
});
}
return meilisearch.search(requests);
return meilisearch.search(requests);
},
},
},
{
onError: () => autocompleteAvailability.setUnavailable(true),
onSuccess: () => autocompleteAvailability.setUnavailable(false),
}
)
{
onError: () => autocompleteAvailability.setUnavailable(true),
onSuccess: () => autocompleteAvailability.setUnavailable(false),
}
)
);
const DEFAULT_DROPDOWN_ITEM_LIMIT = 6;
@@ -136,11 +140,40 @@ const targetData = [
export const AutocompleteSearch = forwardRef<{ focus: () => void }, Props>(({ ...props }, ref) => {
const browsingSettingsAddons = useBrowsingSettingsAddons();
const features = useFeatureFlags();
const [targetIndex, setTargetIndex] = useState<SearchIndexKey>('models');
const handleTargetChange = (value: SearchIndexKey) => {
setTargetIndex(value);
};
const currentUser = useCurrentUser();
// Owned above the keyed search provider below, so it outlives the remount an index switch
// causes.
const carriedSearchText = useRef('');
// Follow the section the user navigates to. This has to live ABOVE the keyed provider for the
// same reason the carrier does: inside it, the effect would run again on the mount that a
// target switch causes, read a section the user has not navigated to, and immediately revert
// their pick — so the category selector would only ever "work" when it picked what the URL
// already said.
const pathname = usePathname();
const currentSection = pathname.split('/')[1] || 'models';
const searchTarget = targetData.find((t) => t.value === currentSection)?.value ?? 'models';
useEffect(() => {
// A navigation is not the switch the carry exists for. The input's blur handler empties the
// visible text WITHOUT emptying the carrier (a blur is how you reach the category selector at
// all), so text a user typed and walked away from would otherwise reappear — and be searched
// again — in the next section they land in.
//
// 🔴 This runs when `searchTarget` CHANGES, which is narrower than "on navigation": the line
// above collapses every first path segment outside `targetData` to `'models'`, so `/` →
// `/models/123/slug`, or any move between two such paths, leaves it unchanged and this never
// runs. The other explicit discard is `blurAndDiscardCarriedText`, on submit and on Escape;
// separately, emptying the input discards through the setter. Together they narrow the window
// rather than closing it, and the remainder is deliberate: text blurred away and then left
// alone survives in the carrier until the next pick from the selector re-seeds it.
carriedSearchText.current = '';
setTargetIndex(searchTarget);
}, [searchTarget]);
const isModels = targetIndex === 'models';
const isImages = targetIndex === 'images';
@@ -165,20 +198,78 @@ export const AutocompleteSearch = forwardRef<{ focus: () => void }, Props>(({ ..
: null,
].filter(isDefined);
const resolvedIndexName = searchIndexMap[targetIndex as keyof typeof searchIndexMap];
// The options the selector OFFERS: every target, narrowed by feature flag. Computed once here
// because the render below reads it twice — as `data`, and in the `value` expression that
// blanks the label when the target is not one of these.
const enabledTargets = targetData.filter(
({ value }) =>
(features.imageSearch ? true : value !== 'images') &&
(features.bounties ? true : value !== 'bounties') &&
(features.articles ? true : value !== 'articles') &&
(features.toolSearch ? true : value !== 'tools') &&
(features.comicSearch ? true : value !== 'comics')
);
return (
<InstantSearch
searchClient={searchClient}
indexName={searchIndexMap[targetIndex as keyof typeof searchIndexMap]}
future={{ preserveSharedStateOnUnmount: false }}
>
<AutocompleteSearchContent
{...props}
indexName={targetIndex}
ref={ref}
onTargetChange={handleTargetChange}
baseFilters={filters}
<Group className={classes.wrapper} gap={0} wrap="nowrap">
{/*
ABOVE the keyed provider, and that placement is the point. `<InstantSearch>` returns `null`
whenever its search instance is not STARTED, and outside server rendering it is started
from a subscription callback that runs after a render has committed — so every fresh
provider renders once with no subtree at all, and a key change builds a fresh provider.
Inside it, the control the user just clicked would be destroyed and rebuilt by their own
click — focus lands on `<body>`. It consumes nothing from the provider's context, so
nothing is lost by lifting it out.
*/}
<Select
// CONTROLLED. Uncontrolled, its displayed label is internal state, so a target switch
// driven from anywhere else — the URL-follow effect above — would leave it showing a
// category the search has moved off: a selector that lies about what it is searching.
//
// `null` rather than the target when the target is not an OFFERED option: the URL can
// point the search at an index whose feature flag is off, and Mantine leaves a
// controlled value it cannot resolve showing the PREVIOUS option's label. Blank is
// honest about "none of these"; a stale label is the same lie in a different place.
value={enabledTargets.some(({ value }) => value === targetIndex) ? targetIndex : null}
aria-label="Search category"
classNames={{
root: classes.targetSelectorRoot,
input: classes.targetSelectorInput,
option: classes.targetSelectorOption,
options: classes.targetSelectorOptions,
dropdown: classes.targetSelectorDropdown,
}}
rightSectionProps={{
className: classes.targetSelectorRightSection,
}}
maxDropdownHeight={280}
data={enabledTargets}
rightSection={<IconChevronDown size={16} color="currentColor" />}
style={{ flexShrink: 1 }}
onChange={(v: string | null) => handleTargetChange(v as SearchIndexKey)}
autoComplete="off"
allowDeselect={false}
/>
</InstantSearch>
<InstantSearch
// Needs re-render, the same way `SearchLayout` does it. Otherwise the search fires with the
// previous index's parameters: react-instantsearch sets the new index and searches in its
// render body, before the children that own `filters` have re-rendered.
key={resolvedIndexName}
searchClient={searchClient}
indexName={resolvedIndexName}
future={{ preserveSharedStateOnUnmount: false }}
>
<AutocompleteSearchContent
{...props}
indexName={targetIndex}
ref={ref}
baseFilters={filters}
carriedSearchText={carriedSearchText}
/>
</InstantSearch>
</Group>
);
});
@@ -186,8 +277,8 @@ AutocompleteSearch.displayName = 'AutocompleteSearch';
type AutocompleteSearchProps<T extends SearchIndexKey> = Props & {
indexName: T;
onTargetChange: (target: T) => void;
baseFilters: string[];
carriedSearchText: React.MutableRefObject<string>;
};
function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
@@ -197,8 +288,8 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
className,
searchBoxProps,
indexName: indexNameProp,
onTargetChange,
baseFilters,
carriedSearchText,
...autocompleteProps
}: AutocompleteSearchProps<TKey>,
ref: React.ForwardedRef<{ focus: () => void }>
@@ -209,11 +300,7 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
const browsingSettingsAddons = useBrowsingSettingsAddons();
const router = useRouter();
const isMobile = useIsMobile();
const features = useFeatureFlags();
const inputRef = useRef<HTMLInputElement>(null);
const pathname = usePathname();
const currentSection = pathname.split('/')[1] || 'models';
const searchTarget = targetData.find((t) => t.value === currentSection)?.value ?? 'models';
const domainColor = useDomainColor();
const { status } = useInstantSearch({
@@ -227,7 +314,7 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
: indexNameProp;
const [selectedItem, setSelectedItem] = useState<ComboboxData[number] | null>(null);
const [search, setSearch] = useState(query);
const [search, setSearch, clearDisplayedText] = useCarriedSearchText(carriedSearchText, query);
const [queryFilters, setQueryFilters] = useState('');
const [debouncedSearch] = useDebouncedValue(search, 300);
@@ -380,6 +467,31 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
const focusInput = () => inputRef.current?.focus();
const blurInput = () => inputRef.current?.blur();
// The two "done" signals that blur this input from here — submitting a search, and pressing
// Escape — discard the carried copy as well as blurring. One function rather than the line
// written at each call site: the two must not drift apart.
//
// 🔴 NOT the whole set of ways a user finishes with what they typed. Picking a hit from the
// dropdown (`handleItemClick` below) navigates and fires the same `onSubmit?.()`, and discards
// nothing of its own. Whether the carrier survives it is then decided by the URL-follow effect
// in the outer component: emptied when the landing path moves `searchTarget`, kept when it does
// not. That is the same residue that effect already describes, not a separate decision taken
// here — so do not read this pair as an exhaustive list.
//
// Deliberately NOT inside `blurInput`, which this is now the only caller of — so the two
// placements are behaviourally identical at this head. `blurInput` is a DOM verb and the
// discard is a claim about intent: put inside it, a future caller that is not a "done" signal
// would inherit the discard silently. (Nothing else blurs at all today: the imperative handle
// below exposes `focus` only.)
//
// 🔴 And NOT from `handleBlur`. Reaching the category selector requires blurring this input, so
// a discard there would empty the carrier immediately before the one switch the carry exists
// for — which is what made the carry inert on this component before.
const blurAndDiscardCarriedText = () => {
carriedSearchText.current = '';
blurInput();
};
useImperativeHandle(ref, () => ({
focus: focusInput,
}));
@@ -391,17 +503,33 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
if (search) {
router.push(searchPageUrl(), undefined, { shallow: false });
blurInput();
// Inside the `if`, where the blur already was: the carrier is discarded on the branch that
// acted on the text. Enter over an EMPTY input — which is what the box reads as after a
// blur, since nothing re-seeds it on focus — discards nothing, and that is the same
// residue the effect above describes.
blurAndDiscardCarriedText();
}
onSubmit?.();
};
// The explicit clear — the input's clear button. The user asked for the text to go, so the
// carrier goes with it.
const handleClear = () => {
setSearch('');
onClear?.();
};
// Blur empties the input the same way it always has, but leaves the carried copy alone. Reaching
// the category selector REQUIRES blurring this input, so a blur that wrote through `setSearch`
// emptied the carrier immediately before every selector-driven index switch — the one path the
// carry exists for. `onClear?.()` still fires, unchanged: on mobile it is what closes the search
// overlay (`AppHeader` passes `onSearchDone`), and that is not ours to change here.
const handleBlur = () => {
clearDisplayedText();
onClear?.();
};
const getItemFromValue = (value: string) => {
return (
items.find((i) => i.value === value) ?? {
@@ -447,7 +575,8 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
useEffect(() => {
// Only set the query when the debounced search changes
// and user didn't select from the list
if (debouncedSearch === query || selectedItem || searchErrorState) return;
if (!shouldRefineSearchQuery(debouncedSearch, query, !!selectedItem || searchErrorState))
return;
// Check if the query is an AIR
const air = checkAIR(indexName, debouncedSearch);
@@ -461,22 +590,18 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
setQuery(cleanedSearch);
setQueryFilters(filters);
// `searchErrorState` is a module-level store, so it is the one input here that SURVIVES the
// remount an index switch causes. Without it in the deps, a tree that remounted while search
// was unavailable restores the typed text, returns early, and then never refines when the
// flag clears — the box reads as populated while the fresh helper's query is still empty.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [debouncedSearch, query, indexName]);
}, [debouncedSearch, query, indexName, searchErrorState]);
// Clear selected item after search changes
useEffect(() => {
setSelectedItem(null);
}, [debouncedSearch]);
// Change index target when search target changes
useEffect(() => {
if (indexNameProp !== searchTarget) {
onTargetChange(searchTarget as TKey);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [searchTarget]);
const processHitUrl = (hit: Hit) => {
switch (indexName) {
case 'articles':
@@ -503,187 +628,155 @@ function AutocompleteSearchContentInner<TKey extends SearchIndexKey>(
filters={[...baseFilters, queryFilters]}
hitsPerPage={DEFAULT_DROPDOWN_ITEM_LIMIT}
/>
<Group className={classes.wrapper} gap={0} wrap="nowrap">
<Select
key={pathname}
aria-label="Search category"
classNames={{
root: classes.targetSelectorRoot,
input: classes.targetSelectorInput,
option: classes.targetSelectorOption,
options: classes.targetSelectorOptions,
dropdown: classes.targetSelectorDropdown,
}}
rightSectionProps={{
className: classes.targetSelectorRightSection,
}}
maxDropdownHeight={280}
defaultValue={searchTarget}
// Ensure we disable search targets if they are not enabled
data={targetData.filter(
({ value }) =>
(features.imageSearch ? true : value !== 'images') &&
(features.bounties ? true : value !== 'bounties') &&
(features.articles ? true : value !== 'articles') &&
(features.toolSearch ? true : value !== 'tools') &&
(features.comicSearch ? true : value !== 'comics')
)}
rightSection={<IconChevronDown size={16} color="currentColor" />}
style={{ flexShrink: 1 }}
onChange={(v: string | null) => onTargetChange(v as TKey)}
autoComplete="off"
allowDeselect={false}
/>
<ClearableAutoComplete
ref={inputRef}
key={indexName}
className={className}
classNames={classes}
placeholder="Search Civitai"
type="search"
limit={
results && results.nbHits > DEFAULT_DROPDOWN_ITEM_LIMIT
? DEFAULT_DROPDOWN_ITEM_LIMIT + 1 // Allow one more to show more results option
: DEFAULT_DROPDOWN_ITEM_LIMIT
<ClearableAutoComplete
ref={inputRef}
key={indexName}
className={className}
classNames={classes}
placeholder="Search Civitai"
type="search"
limit={
results && results.nbHits > DEFAULT_DROPDOWN_ITEM_LIMIT
? DEFAULT_DROPDOWN_ITEM_LIMIT + 1 // Allow one more to show more results option
: DEFAULT_DROPDOWN_ITEM_LIMIT
}
defaultValue={query}
value={search}
data={items}
onChange={(value) => {
if (value == null || value === 'View more results') return;
setSearch(value);
}}
onBlur={handleBlur}
onClear={handleClear}
onKeyDown={getHotkeyHandler([
['Escape', blurAndDiscardCarriedText],
['Enter', handleSubmit],
])}
onOptionSubmit={handleItemClick}
renderOption={({ option }) => {
const { key, ...item } = getItemFromValue(option.value);
// Render special states
if (key === 'blocked') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search query contains inappropriate content and has been blocked.
</Text>
<Text size="xs" align="center">
Please try a different search term.
</Text>
</Stack>
);
}
defaultValue={query}
value={search}
data={items}
onChange={(value) => {
if (value == null || value === 'View more results') return;
setSearch(value);
}}
onBlur={handleClear}
onClear={handleClear}
onKeyDown={getHotkeyHandler([
['Escape', blurInput],
['Enter', handleSubmit],
])}
onOptionSubmit={handleItemClick}
renderOption={({ option }) => {
const { key, ...item } = getItemFromValue(option.value);
// Render special states
if (key === 'blocked') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search query contains inappropriate content and has been blocked.
if (key === 'profanity') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search query contains inappropriate content that violates our community
guidelines.
</Text>
{profanityAnalysis.matches.length > 0 && (
<Text size="xs" align="center" c="dimmed">
Flagged terms: {profanityAnalysis.matches.join(', ')}
</Text>
<Text size="xs" align="center">
Please try a different search term.
</Text>
</Stack>
);
}
if (key === 'profanity') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search query contains inappropriate content that violates our community
guidelines.
</Text>
{profanityAnalysis.matches.length > 0 && (
<Text size="xs" align="center" c="dimmed">
Flagged terms: {profanityAnalysis.matches.join(', ')}
</Text>
)}
<Text size="xs" align="center">
Please refine your search terms to find appropriate content.
</Text>
</Stack>
);
}
if (key === 'disabled') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search includes terms tied to real people. Content depicting real people is
filtered from search results.
</Text>
</Stack>
);
}
if (key === 'blocked-words') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search query contains blocked words and has been filtered.
</Text>
<Text size="xs" align="center">
Please try a different search term.
</Text>
</Stack>
);
}
if (key === 'error') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
There was an error while performing your request&hellip;
</Text>
<Text size="xs" align="center">
Please try again later
</Text>
</Stack>
);
}
)}
<Text size="xs" align="center">
Please refine your search terms to find appropriate content.
</Text>
</Stack>
);
}
if (key === 'disabled') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search includes terms tied to real people. Content depicting real people is
filtered from search results.
</Text>
</Stack>
);
}
if (key === 'blocked-words') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
Your search query contains blocked words and has been filtered.
</Text>
<Text size="xs" align="center">
Please try a different search term.
</Text>
</Stack>
);
}
if (key === 'error') {
return (
<Stack gap="xs" align="center">
<Text size="sm" align="center">
There was an error while performing your request&hellip;
</Text>
<Text size="xs" align="center">
Please try again later
</Text>
</Stack>
);
}
const Render = IndexRenderItem[indexName] ?? ModelSearchItem;
return <Render {...item} />;
}}
rightSection={
<HoverCard withArrow width={300} shadow="sm" openDelay={500}>
<HoverCard.Target>
<Text
component="div"
role="button"
tabIndex={0}
aria-label="Quick search keyboard shortcut"
fw="bold"
style={{
border: `1px solid ${
colorScheme === 'dark' ? theme.colors.dark[4] : theme.colors.gray[3]
}`,
borderRadius: theme.radius.sm,
backgroundColor:
colorScheme === 'dark' ? theme.colors.dark[7] : theme.colors.gray[0],
color: colorScheme === 'dark' ? theme.colors.gray[5] : theme.colors.gray[6],
textAlign: 'center',
width: 24,
userSelect: 'none',
}}
>
/
</Text>
</HoverCard.Target>
<HoverCard.Dropdown>
<Text size="sm" c="yellow" fw={500}>
Pro-tip: Quick search faster!
</Text>
<Text size="xs" lh={1.2}>
Open the quick search without leaving your keyboard by tapping the <Code>/</Code>{' '}
key from anywhere and just start typing.
</Text>
</HoverCard.Dropdown>
</HoverCard>
}
// prevent default filtering behavior
filter={({ options }) => options}
clearable={query.length > 0}
maxDropdownHeight={isMobile ? 'calc(90vh - var(--header-height))' : 500}
{...autocompleteProps}
/>
<LegacyActionIcon
className={classes.searchButton}
color="gray"
variant="filled"
size={36}
onMouseDown={handleSubmit}
aria-label="Search"
>
<IconSearch size={18} />
</LegacyActionIcon>
</Group>
const Render = IndexRenderItem[indexName] ?? ModelSearchItem;
return <Render {...item} />;
}}
rightSection={
<HoverCard withArrow width={300} shadow="sm" openDelay={500}>
<HoverCard.Target>
<Text
component="div"
role="button"
tabIndex={0}
aria-label="Quick search keyboard shortcut"
fw="bold"
style={{
border: `1px solid ${
colorScheme === 'dark' ? theme.colors.dark[4] : theme.colors.gray[3]
}`,
borderRadius: theme.radius.sm,
backgroundColor:
colorScheme === 'dark' ? theme.colors.dark[7] : theme.colors.gray[0],
color: colorScheme === 'dark' ? theme.colors.gray[5] : theme.colors.gray[6],
textAlign: 'center',
width: 24,
userSelect: 'none',
}}
>
/
</Text>
</HoverCard.Target>
<HoverCard.Dropdown>
<Text size="sm" c="yellow" fw={500}>
Pro-tip: Quick search faster!
</Text>
<Text size="xs" lh={1.2}>
Open the quick search without leaving your keyboard by tapping the <Code>/</Code>{' '}
key from anywhere and just start typing.
</Text>
</HoverCard.Dropdown>
</HoverCard>
}
// prevent default filtering behavior
filter={({ options }) => options}
clearable={query.length > 0}
maxDropdownHeight={isMobile ? 'calc(90vh - var(--header-height))' : 500}
{...autocompleteProps}
/>
<LegacyActionIcon
className={classes.searchButton}
color="gray"
variant="filled"
size={36}
onMouseDown={handleSubmit}
aria-label="Search"
>
<IconSearch size={18} />
</LegacyActionIcon>
</>
);
}
@@ -700,4 +793,3 @@ const IndexRenderItem: Record<SearchIndexKey, React.ComponentType<any>> = {
tools: ToolSearchItem,
comics: ComicsSearchItem,
};
src/components/Search/QuickSearchDropdown.tsx
+159 -105
View File
@@ -28,6 +28,10 @@ import type { ShowcaseItemSchema } from '~/server/schema/user-profile.schema';
import { paired } from '~/utils/type-guards';
import { searchClient } from '~/components/Search/search.client';
import { createResilientSearchClient } from '~/components/Search/resilientSearchClient';
import {
shouldRefineSearchQuery,
useCarriedSearchText,
} from '~/components/Search/useCarriedSearchText';
import { BrowsingLevelFilter } from './CustomSearchComponents';
import { ToolSearchItem } from '~/components/AutocompleteSearch/renderItems/tools';
import { ComicsSearchItem } from '~/components/AutocompleteSearch/renderItems/comics';
@@ -151,60 +155,135 @@ export const QuickSearchDropdown = ({
dropdownItemLimit = 5,
startingIndex,
disableInitialSearch,
showIndexSelect = true,
...props
}: QuickSearchDropdownProps) => {
const [targetIndex, setTargetIndex] = useState<SearchIndexKey>(startingIndex ?? 'models');
const features = useFeatureFlags();
// The target is clamped to `supportedIndexes` — the set the CALLER declared, which is not
// necessarily the set the selector offers: the offered list below narrows it further by feature
// flag, and this fallback does not. A bare `models` fallback would leave the component searching
// an index the caller never supported — a caller that passes `['users']` gets a users picker
// whose hits are models.
//
// Every current caller either passes `startingIndex` or supports `models` first, so the
// INITIAL value below is unchanged at every call site today. The reachable path is the
// deselect one: Mantine's single-select is deselectable, so `onChange` can hand the change
// handler `null`, and a bare `'models'` fallback would then move a `supportedIndexes={['users']}`
// picker onto the models index.
const fallbackIndex = startingIndex ?? props.supportedIndexes?.[0] ?? 'models';
const [targetIndex, setTargetIndex] = useState<SearchIndexKey>(fallbackIndex);
const handleTargetChange = (value: SearchIndexKey | null) => {
setTargetIndex(value ?? 'models');
setTargetIndex(value ?? fallbackIndex);
};
// Owned above the keyed search provider below, so it outlives the remount an index switch
// causes.
const carriedSearchText = useRef('');
const indexName = searchIndexMap[targetIndex];
return (
<InstantSearch
searchClient={disableInitialSearch ? searchClient : meilisearch}
indexName={indexName}
future={{ preserveSharedStateOnUnmount: true }}
>
<BrowsingLevelFilter
indexKey={targetIndex}
filters={filters}
hitsPerPage={dropdownItemLimit}
/>
// The options the selector OFFERS: what the caller declared, narrowed by feature flag. Computed
// once here because the render below reads it twice — as `data`, and in the `value` expression
// that blanks the label when the target is not one of these.
//
// Not the same set as the one `fallbackIndex` above falls back into: that one stops at
// `supportedIndexes` and is deliberately NOT narrowed by flag, so a flag-disabled
// `startingIndex` reaches `targetIndex` and the `value` expression blanks the label rather than
// the fallback rewriting the target.
const enabledTargets = (props.supportedIndexes ?? [])
.filter(
(value) =>
(features.imageSearch ? true : searchIndexMap[value] !== IMAGES_SEARCH_INDEX) &&
(features.toolSearch ? true : searchIndexMap[value] !== TOOLS_SEARCH_INDEX) &&
(features.articles ? true : value !== 'articles')
)
.map((index) => ({ label: IndexToLabel[searchIndexMap[index]], value: index }));
<QuickSearchDropdownContent
{...props}
indexName={targetIndex}
onIndexNameChange={handleTargetChange}
dropdownItemLimit={dropdownItemLimit}
/>
</InstantSearch>
return (
<Group className={classes.wrapper} gap={0} wrap="nowrap">
{!!showIndexSelect && (
/*
ABOVE the keyed provider, and that placement is the point. `<InstantSearch>` returns
`null` whenever its search instance is not STARTED, and outside server rendering it is
started from a subscription callback that runs after a render has committed — so every
fresh provider renders once with no subtree at all, and a key change builds a fresh
provider. Inside it, the control the user just clicked would be destroyed and rebuilt by
their own click — focus lands on `<body>`. It consumes nothing from the provider's
context, so nothing is lost by lifting it out.
*/
<Select
className="shrink"
classNames={{
root: classes.targetSelectorRoot,
input: classes.targetSelectorInput,
section: classes.targetSelectorRightSection,
}}
maxDropdownHeight={280}
// CONTROLLED — and the reason this comment used to give is gone, with no replacement
// established. That reason was that the selector sat inside the keyed provider, so a
// target switch remounted it and reset its internal state; lifting it above the provider
// removed the mechanism. `targetIndex` in this component now has exactly one writer,
// `handleTargetChange`, reached only from this Select's own `onChange` — so there is no
// second source for a displayed label to drift away from. Do not read the sibling in
// `AutocompleteSearch` as agreeing: that one has a second writer (a URL-follow effect),
// and its identical prop is load-bearing for the reason stated there.
//
// What does still need a `value` is the expression below, which has no uncontrolled
// equivalent: `null` rather than the target when the target is not an OFFERED option.
// Mantine leaves a controlled value it cannot resolve showing the PREVIOUS option's
// label, which is a lie about what is being searched; blank is honest about "none of
// these". Reachable, because `fallbackIndex` above is not flag-narrowed.
value={enabledTargets.some(({ value }) => value === targetIndex) ? targetIndex : null}
data={enabledTargets}
rightSection={<IconChevronDown size={16} color="currentColor" />}
onChange={(value) => handleTargetChange(value as SearchIndexKey)}
/>
)}
<InstantSearch
// Needs re-render, the same way `SearchLayout` does it. Otherwise the search fires with the
// previous index's parameters: react-instantsearch sets the new index and searches in its
// render body, before the children that own `filters` have re-rendered.
key={indexName}
searchClient={disableInitialSearch ? searchClient : meilisearch}
indexName={indexName}
future={{ preserveSharedStateOnUnmount: true }}
>
<BrowsingLevelFilter
indexKey={targetIndex}
filters={filters}
hitsPerPage={dropdownItemLimit}
/>
<QuickSearchDropdownContent
{...props}
indexName={targetIndex}
dropdownItemLimit={dropdownItemLimit}
carriedSearchText={carriedSearchText}
/>
</InstantSearch>
</Group>
);
};
function QuickSearchDropdownContent<TIndex extends SearchIndexKey>({
indexName: indexNameProp,
onIndexNameChange,
onItemSelected,
filters,
supportedIndexes,
dropdownItemLimit = 5,
showIndexSelect = true,
placeholder,
onHits,
carriedSearchText,
...autocompleteProps
}: QuickSearchDropdownProps & {
indexName: TIndex;
onIndexNameChange: (indexName: TIndex) => void;
carriedSearchText: React.MutableRefObject<string>;
}) {
// const currentUser = useCurrentUser();
const { query, refine: setQuery, isSearchStalled } = useSearchBox();
const { hits, results } = useHitsTransformed<TIndex>();
const features = useFeatureFlags();
const [search, setSearch] = useState(query);
const [search, setSearch] = useCarriedSearchText(carriedSearchText, query);
const [debouncedSearch] = useDebouncedValue(search, 300);
const isSubmittingOptionRef = useRef(false);
const availableIndexes = supportedIndexes ?? [];
const indexName = results?.index
? reverseSearchIndexMap[results.index as ReverseSearchIndexKey]
@@ -275,7 +354,7 @@ function QuickSearchDropdownContent<TIndex extends SearchIndexKey>({
useEffect(() => {
// Only set the query when the debounced search changes
// and user didn't select from the list
if (debouncedSearch === query) return;
if (!shouldRefineSearchQuery(debouncedSearch, query)) return;
setQuery(debouncedSearch);
// eslint-disable-next-line react-hooks/exhaustive-deps
@@ -286,87 +365,62 @@ function QuickSearchDropdownContent<TIndex extends SearchIndexKey>({
const loading = search.length > 0 && (search !== query || isSearchStalled);
return (
<Group className={classes.wrapper} gap={0} wrap="nowrap">
{!!showIndexSelect && (
<Select
className="shrink"
classNames={{
root: classes.targetSelectorRoot,
input: classes.targetSelectorInput,
section: classes.targetSelectorRightSection,
}}
maxDropdownHeight={280}
defaultValue={availableIndexes[0]}
// Ensure we disable search targets if they are not enabled
data={availableIndexes
.filter(
(value) =>
(features.imageSearch ? true : searchIndexMap[value] !== IMAGES_SEARCH_INDEX) &&
(features.toolSearch ? true : searchIndexMap[value] !== TOOLS_SEARCH_INDEX) &&
(features.articles ? true : value !== 'articles')
)
.map((index) => ({ label: IndexToLabel[searchIndexMap[index]], value: index }))}
rightSection={<IconChevronDown size={16} color="currentColor" />}
onChange={(value) => onIndexNameChange(value as TIndex)}
/>
)}
<ClearableAutoComplete
key={indexName}
classNames={classes}
placeholder={placeholder ?? 'Search Civitai'}
type="search"
maxDropdownHeight={300}
// TODO: Mantine7
// nothingFound={
// !hits.length ? (
// <Stack gap={0} align="center">
// <TimeoutLoader delay={1500} renderTimeout={() => <Text>No results found</Text>} />
// </Stack>
// ) : undefined
// }
limit={
results && results.nbHits > dropdownItemLimit
? dropdownItemLimit + 1 // Allow one more to show more results option
: dropdownItemLimit
<ClearableAutoComplete
key={indexName}
classNames={classes}
placeholder={placeholder ?? 'Search Civitai'}
type="search"
maxDropdownHeight={300}
// TODO: Mantine7
// nothingFound={
// !hits.length ? (
// <Stack gap={0} align="center">
// <TimeoutLoader delay={1500} renderTimeout={() => <Text>No results found</Text>} />
// </Stack>
// ) : undefined
// }
limit={
results && results.nbHits > dropdownItemLimit
? dropdownItemLimit + 1 // Allow one more to show more results option
: dropdownItemLimit
}
defaultValue={query}
value={search}
data={items}
onChange={(value) => {
// Ignore onChange events that happen during option submission
if (isSubmittingOptionRef.current) {
isSubmittingOptionRef.current = false;
return;
}
defaultValue={query}
value={search}
data={items}
onChange={(value) => {
// Ignore onChange events that happen during option submission
if (isSubmittingOptionRef.current) {
isSubmittingOptionRef.current = false;
return;
}
setSearch(value);
}}
onClear={() => setSearch('')}
// onBlur={() => (!isMobile ? onClear?.() : undefined)}
onOptionSubmit={(value) => {
const item = getItemFromValue(value);
if (item) {
// Set flag before calling onItemSelected to prevent onChange from overwriting
isSubmittingOptionRef.current = true;
setSearch(value);
}}
onClear={() => setSearch('')}
// onBlur={() => (!isMobile ? onClear?.() : undefined)}
onOptionSubmit={(value) => {
const item = getItemFromValue(value);
if (item) {
// Set flag before calling onItemSelected to prevent onChange from overwriting
isSubmittingOptionRef.current = true;
onItemSelected(
{
entityId: item.hit.id,
entityType: SearchIndexEntityTypes[searchIndexMap[indexName]],
},
item.hit as any
);
onItemSelected(
{
entityId: item.hit.id,
entityType: SearchIndexEntityTypes[searchIndexMap[indexName]],
},
item.hit as any
);
setSearch('');
}
}}
renderOption={renderOption}
// prevent default filtering behavior
filter={({ options }) => options}
clearable={query.length > 0}
loading={loading}
{...autocompleteProps}
/>
</Group>
setSearch('');
}
}}
renderOption={renderOption}
// prevent default filtering behavior
filter={({ options }) => options}
clearable={query.length > 0}
loading={loading}
{...autocompleteProps}
/>
);
}
src/components/Search/__tests__/dropdown-index-remount.test.ts
+799
View File
@@ -0,0 +1,799 @@
// @vitest-environment happy-dom
import type { Dirent } from 'fs';
import { readdirSync, readFileSync } from 'fs';
import path from 'path';
import * as React from 'react';
import type { act as actType } from 'react-dom/test-utils';
import { createRoot, type Root } from 'react-dom/client';
import { afterEach, describe, expect, it } from 'vitest';
import {
seedCarriedSearchText,
shouldRefineSearchQuery,
useCarriedSearchText,
} from '~/components/Search/useCarriedSearchText';
// React 18.3 exposes `act` on the `react` export, but our @types/react (18.0.x) predates that
// typing. Use the runtime `React.act` and borrow the signature from react-dom/test-utils — the
// same arrangement the other node-tier React tests in this repo use.
const act = (React as unknown as { act: typeof actType }).act;
const repoRoot = path.resolve(__dirname, '../../../..');
const read = (relPath: string) => readFileSync(path.join(repoRoot, relPath), 'utf8');
/**
* Drop comments before any check that COUNTS or LOCATES a token. Prose naming the token satisfies
* it otherwise — including, in this file's case, prose written to warn against the mutation the
* count exists to catch. Whole-line `//` only, plus block comments, so a `//` inside a string
* literal cannot blind the scan.
*/
const stripComments = (source: string) =>
source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, '');
/**
* The selector's value, in full. The predicate alone says WHICH list is consulted; the two
* branches say what is done with the answer, and both are load-bearing — negating the condition
* shows the target only when it is NOT offered, and replacing the `null` with a first-option
* fallback reinstates the wrong-label lie the clamp exists to prevent. Neither is visible to a
* check on the `.some(…)` call.
*
* Compared with `containsIgnoringWhitespace`, never `toContain`. This is one expression against
* `printWidth: 100`, and it has already sat at EXACTLY 100 characters once: a rename, or one more
* level of indentation, makes prettier re-wrap it and a literal check then goes red about a clamp
* that is still correct. Only the whitespace is forgiven — every identifier, operator and branch
* still has to be there, in order. Re-pin a renamed spelling; do not loosen it to the predicate.
*/
const SELECTOR_VALUE_CLAMP =
'value={enabledTargets.some(({ value }) => value === targetIndex) ? targetIndex : null}';
/**
* Containment with every whitespace character removed from BOTH sides. Prettier breaks a long JSX
* attribute inside its own braces as well as between attributes, so collapsing runs to a single
* space does not survive a wrap — removing whitespace entirely does. What that gives up is real,
* but does not reach THIS needle: with whitespace stripped from both sides, sources differing only
* inside a string literal compare equal, and a needle whose tokens are separated only by a space
* matches a source that has run them together. `SELECTOR_VALUE_CLAMP` contains no string literal,
* and every adjacency in it is punctuated rather than whitespace-separated, so neither applies.
* Do not reuse this for a needle that does contain a literal.
*/
const containsIgnoringWhitespace = (source: string, needle: string) =>
source.replace(/\s+/g, '').includes(needle.replace(/\s+/g, ''));
/**
* Every `<InstantSearch>` root in the app, and what each is required to do about the index it
* targets. `keyed` roots take a target the user can change at runtime: react-instantsearch-core
* calls `helper.setIndex(indexName).search()` in its RENDER body, and the provider renders before
* the children that own `filters`, so a target switch on an unkeyed root searches the NEW index
* with the PREVIOUS target's parameters. `key` makes React build a fresh provider instead.
*
* The ledger is exhaustive on purpose: a new root added without a decision fails this rather than
* inheriting a default.
*/
const INSTANT_SEARCH_ROOTS = {
'src/components/Search/SearchLayout.tsx': 'keyed',
'src/components/AutocompleteSearch/AutocompleteSearch.tsx': 'keyed',
'src/components/Search/QuickSearchDropdown.tsx': 'keyed',
// Exempt, and the reason is asserted below rather than taken on trust: its index is a fixed
// member of `searchIndexMap`, so there is no switch for a stale parameter set to survive.
'src/components/CollectionSelectModal/CollectionSelectModal.tsx': 'static-index',
} as const;
/**
* Every non-test `.tsx` under `src/` that renders an `<InstantSearch>` element, repo-relative.
*
* A full-suite run creates and removes directories under `src/` while this walk is happening, so
* an entry can vanish between the listing and the read. That is an OBSERVED hazard, not a
* hypothetical one, and it surfaces as a COLLECTION failure — which contributes zero tests and
* moves no failure count, so it reads as "nothing to see". Entries that cannot be read are
* therefore skipped rather than thrown on.
*/
function findInstantSearchRoots(dir = 'src'): string[] {
const found: string[] = [];
let entries: Dirent[];
try {
entries = readdirSync(path.join(repoRoot, dir), { withFileTypes: true });
} catch {
return found;
}
for (const entry of entries) {
const relPath = `${dir}/${entry.name}`;
if (entry.isDirectory()) {
found.push(...findInstantSearchRoots(relPath));
} else if (entry.name.endsWith('.tsx') && !entry.name.includes('.test.')) {
try {
if (stripComments(read(relPath)).includes('<InstantSearch')) found.push(relPath);
} catch {
continue;
}
}
}
return found;
}
/** The opening `<InstantSearch …>` tag. Callers pass `stripComments`ed source. */
function openingTag(source: string): string {
// The ELEMENT, not a mention of it: prose naming the tag matches the same pattern, so the tag is
// identified by the prop every root must pass rather than by its name alone.
const tags = [...source.matchAll(/<InstantSearch\b[^>]*>/g)]
.map((m) => m[0])
.filter((tag) => /\ssearchClient=\{/.test(tag));
if (tags.length !== 1)
throw new Error(`expected one <InstantSearch> element, found ${tags.length}`);
return tags[0];
}
function propExpression(tag: string, prop: string): string | null {
const match = tag.match(new RegExp(`(?:^|\\s)${prop}=\\{([^}]*)\\}`));
return match ? match[1].trim() : null;
}
/**
* The index a dropdown root passes must be DERIVED FROM ITS TARGET, not pinned to a constant —
* pinning it leaves `key` and `indexName` still agreeing (so the ledger below still passes) while
* the target selector stops switching index at all in production.
*
* Stated as what a tracking index IS — subscripted by `targetIndex` — rather than as a list of
* constant spellings to reject: enumerating spellings caught `searchIndexMap.models` and a string
* literal while `searchIndexMap['models']` and an imported `IMAGES_SEARCH_INDEX` walked straight
* through. Matched against the EXPRESSION, so it holds whether that expression is written inline
* on the provider or hoisted into a local first. Anchored, so it also rejects a WRAPPED form such
* as `useMemo(() => searchIndexMap[targetIndex], …)` — correct code, declined rather than
* accommodated: memoising a hash lookup is not worth widening a guard for, and the failure is
* legible if anyone ever does it.
*
* Scoped to the two dropdowns on purpose. `SearchLayout` takes its index as a PROP — dynamic by
* construction — and an earlier attempt to resolve identifiers generically bound its name to an
* unrelated `const indexName = Object.keys(uiState)?.[0]` elsewhere in that file.
*/
const INDEX_TRACKS_TARGET_EXPRESSION = /^searchIndexMap\[\s*targetIndex\b/;
/**
* The category `<Select>` element's source, delimited by the provider that follows it. Both
* dropdowns render the selector immediately above `<InstantSearch>` — the file-order check below
* is what keeps that true — and a brace-counting parse is not worth writing for it: over-reading
* to the provider can only make a `not.toMatch` on this region WIDER, never blinder.
*/
function selectorSource(source: string): string {
const selector = source.indexOf('<Select');
const provider = source.indexOf('<InstantSearch');
if (selector < 0 || provider < selector)
throw new Error('expected a <Select> written above <InstantSearch>');
return source.slice(selector, provider);
}
/**
* The expression a provider receives, resolved one hop when it is a hoisted local `const` — which
* is how the dropdowns write it. Both the inline and the hoisted form are correct code, so a check
* that reads only one of them rejects the other; this is what lets the callers assert the SHAPE
* without also dictating where it is written.
*
* Only ever called with a bare identifier (the caller tests for that), so nothing here needs
* regex-escaping. The optional `:type` tolerates an annotated declaration.
*/
function resolveOneHop(source: string, expression: string): string {
if (!/^[A-Za-z_$][\w$]*$/.test(expression)) return expression;
const declaration = source.match(new RegExp(`const ${expression}\\s*(?::[^=]+)?=\\s*([^;\\n]+)`));
return declaration?.[1].trim() ?? expression;
}
describe('the InstantSearch roots', () => {
it('is the set this ledger accounts for', () => {
// Derived from the tree, so the ledger fails when the population grows OR shrinks — a new
// root cannot be added without deciding what it does about a changing index.
expect(findInstantSearchRoots().sort()).toEqual(Object.keys(INSTANT_SEARCH_ROOTS).sort());
});
for (const [relPath, policy] of Object.entries(INSTANT_SEARCH_ROOTS)) {
if (policy === 'static-index') {
it(`${relPath} targets a fixed index, so it needs no key`, () => {
const tag = openingTag(stripComments(read(relPath)));
expect(propExpression(tag, 'indexName')).toMatch(/^searchIndexMap\.[A-Za-z]+$/);
expect(propExpression(tag, 'key')).toBeNull();
});
continue;
}
it(`${relPath} keys its provider on the very expression it passes as indexName`, () => {
const source = stripComments(read(relPath));
const tag = openingTag(source);
const indexName = propExpression(tag, 'indexName');
const key = propExpression(tag, 'key');
// Not merely "a key is present": the key and the index have to be the SAME expression, or
// they can disagree and the provider survives a switch it was supposed to be rebuilt for.
expect(indexName).toBeTruthy();
expect(key).toBe(indexName);
// 🔴 THERE IS DELIBERATELY NOTHING MORE HERE FOR `SearchLayout`, AND THAT IS A DECISION.
// Four review rounds were spent on a guard requiring its index expression to reference the
// prop, each round's fix producing the next round's finding: it passed broken code twice
// (an imported constant; a shadowing local behind a comment) and rejected correct code three
// times (a normalisation hop, the `export const` style, a reordered destructuring), and the
// last version lost a kill while adding brittleness. It was a regex approximation of scope
// resolution, and it never reached a fixed point.
//
// The requirement did not survive being questioned: `SearchLayout` is not touched by the
// change this file was written for, so that guard protected an invariant no commit here can
// violate, at the cost of reddening ordinary refactors with a message that misdiagnosed them.
// `key === indexName` above is order-, style- and alias-independent, and is the claim this
// ledger exists to make. Do not add it back without a defect it would have caught.
});
}
});
describe('the dropdown roots carry the typed text across that remount', () => {
// Keying the provider remounts the subtree the typed text lives in, so each of these two has to
// hold that text ABOVE the provider. These are SPELLING-AND-FILE-ORDER checks, not tree-position
// ones: they pin that the declaration is written before the provider in the same file and that
// the ref is threaded and read by name. A rename, or moving the content component above the
// provider in the file, breaks them for a non-defect. They are the only coverage available here
// — the components are browser-tier, and the node project collects `.test.ts` only. The carry
// MECHANISM's behaviour is exercised further down, against a real remount.
const dropdowns = [
'src/components/AutocompleteSearch/AutocompleteSearch.tsx',
'src/components/Search/QuickSearchDropdown.tsx',
];
for (const relPath of dropdowns) {
it(`${relPath} holds the text above the keyed boundary and seeds the input from it`, () => {
const source = stripComments(read(relPath));
const carrierDeclaration = source.indexOf("const carriedSearchText = useRef('')");
const provider = source.indexOf('<InstantSearch');
expect(carrierDeclaration).toBeGreaterThan(-1);
expect(provider).toBeGreaterThan(carrierDeclaration);
expect(source).toContain('carriedSearchText={carriedSearchText}');
// The hook's RESULT has to drive the input, not merely be called — declared here, and then
// WIRED to the input below. Calling it and seeding from `useState(query)` beside it, or
// leaving the declaration in place and rendering `value={query}`, each revert the whole
// mechanism while a check on the call alone stays green.
//
// The third binding is optional because only one of the two takes it: `AutocompleteSearch`
// needs the display-only clear for its blur handler, and `QuickSearchDropdown` has no blur
// clear to give it to. Both spellings are correct; pinning one would reject the other.
expect(source).toMatch(
/const \[search, setSearch(?:, clearDisplayedText)?\] = useCarriedSearchText\(\s*carriedSearchText,\s*query\s*\)/
);
expect(source).toContain('value={search}');
expect(source).toContain('setSearch(value)');
});
}
it('both dropdowns derive the index they key on from the target', () => {
// INVARIANT GUARD, not regression coverage — and it became one when the check learned to
// accept the inline form: both roots already derived their index correctly at the PR base, so
// this is green there. What the base lacked was the `key`, which the ledger above covers. This
// exists because keying the provider makes a constant index silently survivable: `key` and
// `indexName` would still agree while the selector stopped switching anything.
//
// Follows the expression the PROVIDER actually receives, then resolves it one hop if it is a
// hoisted identifier — which is how both roots write it today. Both halves are needed and
// neither is sufficient: checking only the declaration lets the JSX be pinned to a constant
// while an unused tracking `const` sits above it, and checking only the JSX rejects the
// equally correct inline form.
for (const relPath of dropdowns) {
const source = stripComments(read(relPath));
const expression = propExpression(openingTag(source), 'indexName') ?? '';
expect(resolveOneHop(source, expression), relPath).toMatch(INDEX_TRACKS_TARGET_EXPRESSION);
}
});
it('picking a category reaches the state the index is derived from', () => {
// The counterpart of the input wiring above, and the same hole one level up: the suite pins
// the selector's value, its options and its deselect behaviour, but nothing pinned that
// choosing an option arrives at `setTargetIndex`. Neutering either handler leaves manual
// category switching dead while `AutocompleteSearch` still looks alive — its URL-follow effect
// keeps calling `setTargetIndex` — and the exactly-one-writer count further down cannot see
// it, because that count matches `setTargetIndex(searchTarget` while the selector's handler
// writes `setTargetIndex(value)`.
// ⚠️ Every assertion here except `setTargetIndex(value ?? fallbackIndex)` is an INVARIANT
// GUARD: an equivalent handler chain is present at the PR base. Keying the provider is what
// put it at risk — a consolidation of the two `setTargetIndex` writers this change created
// would take one of them out — so it is worth pinning, but the red-at-base of this test is
// attributable to the fallback spelling alone, not to the claim in its title.
//
// The selector now lives in the same component as the handler (it was lifted out of the keyed
// subtree so a key change cannot destroy the control mid-click), so the chain is
// `onChange` → `handleTargetChange` → `setTargetIndex`. The hop the lift removed is the one
// that crossed the component boundary: at the PR base the inner component received an
// `onTargetChange` prop and the chain ran through it.
const autocomplete = stripComments(
read('src/components/AutocompleteSearch/AutocompleteSearch.tsx')
);
expect(autocomplete).toContain(
'onChange={(v: string | null) => handleTargetChange(v as SearchIndexKey)}'
);
expect(autocomplete).toMatch(
/const handleTargetChange = \(value: SearchIndexKey\) => \{\s*setTargetIndex\(value\);\s*\};/
);
const quickSearch = stripComments(read('src/components/Search/QuickSearchDropdown.tsx'));
expect(quickSearch).toContain(
'onChange={(value) => handleTargetChange(value as SearchIndexKey)}'
);
expect(quickSearch).toContain('setTargetIndex(value ?? fallbackIndex)');
});
it('both selectors are rendered ABOVE the provider a target switch rebuilds', () => {
// `<InstantSearch>` returns `null` whenever its search instance is not STARTED, and it is
// started from a subscription callback that runs after a render has committed — so every
// fresh provider, a key change included, renders once with NO subtree at all. A selector
// inside it is therefore unmounted and rebuilt by the very click that switched the index, and
// the focus that click put on it lands on `<body>`. Above the provider it survives its own
// change handler.
//
// File-order, like the carrier check above: a spelling-and-position claim, not a tree one.
// That is what this tier can see, and it is the property that broke.
for (const relPath of dropdowns) {
const source = stripComments(read(relPath));
const selector = source.indexOf('<Select');
const provider = source.indexOf('<InstantSearch');
expect(selector, `${relPath}: no <Select> found`).toBeGreaterThan(-1);
expect(provider, `${relPath}: <Select> is not written above <InstantSearch>`).toBeGreaterThan(
selector
);
}
});
it('both refine gates go through the one predicate, negation included', () => {
// The leading `!` is the whole gate. Dropping it inverts both effects — they return early
// exactly when they should refine — which kills the feature with every other assertion here
// satisfied, because a check on the call expression alone cannot see the operator in front
// of it. Pinned as the complete `if`, per component, since their blocked arguments differ.
// `toMatch` on this one, not `toContain`: prettier wraps it across two lines, so the `return`
// it guards has to be matched across the break — otherwise neutering the consequent leaves the
// effect refining during an outage with the gate itself still spelled correctly.
expect(stripComments(read('src/components/AutocompleteSearch/AutocompleteSearch.tsx'))).toMatch(
/if \(!shouldRefineSearchQuery\(debouncedSearch, query, !!selectedItem \|\| searchErrorState\)\)\s*return;/
);
expect(stripComments(read('src/components/Search/QuickSearchDropdown.tsx'))).toContain(
'if (!shouldRefineSearchQuery(debouncedSearch, query)) return;'
);
});
it('AutocompleteSearch follows the URL section from ABOVE the keyed provider', () => {
// MEASURED REGRESSION, not a hypothetical. This sync used to live inside the subtree with
// `[searchTarget]` deps, and `searchTarget` comes from the pathname rather than from the pick.
// A mount runs every effect, so once the provider is keyed, choosing a category remounts the
// subtree, the effect sees the URL's section instead of the pick, and reverts it — the header
// category selector then only ever "works" when it picks what the URL already said.
//
// Comments stripped: every check below counts or locates a token, and prose naming the token
// — including prose warning against the very mutation being counted — would satisfy it.
const source = stripComments(read('src/components/AutocompleteSearch/AutocompleteSearch.tsx'));
const sync = source.indexOf('setTargetIndex(searchTarget)');
const provider = source.indexOf('<InstantSearch');
expect(sync).toBeGreaterThan(-1);
expect(provider).toBeGreaterThan(sync);
// EXACTLY ONE writer. Hoisting the sync while leaving the old copy in place reintroduces the
// whole defect with the assertion above still satisfied — the consolidation-that-forgot-to-
// delete shape, which is the likeliest way this comes back. Note that the `indexOf` check
// above does NOT screen that shape out: the hoisted copy is still present, so it succeeds and
// execution reaches this line. This count is the only thing that sees the leftover.
//
// Counted on the ARGUMENT, not on a list of callee spellings. A leftover copy is spelled with
// whatever name the boundary it crossed carried — at the PR base that was
// `onTargetChange(searchTarget as TKey)`, plumbed as a prop into the inner component — so an
// alternation of the names known today goes blind the moment a third one appears. Any call
// taking `searchTarget` is counted instead; today the sync itself is the only one. The cost is
// real and accepted: a future LEGITIMATE reader of `searchTarget` reddens this too. Re-pin it
// deliberately then — do not loosen the count back to one spelling.
expect(
[...source.matchAll(/[A-Za-z_$][\w$]*\(\s*searchTarget\b/g)].map((m) => m[0]),
'AutocompleteSearch: expected exactly one call taking `searchTarget` — a second is the ' +
'leftover copy a hoist forgot to delete'
).toHaveLength(1);
// …and it still FOLLOWS navigation. Emptying its dependency array leaves one writer, in the
// right place, that only ever runs once. The array only has to CONTAIN `searchTarget` —
// requiring it to be exactly `[searchTarget]` would go red on a legitimate added dependency.
expect(source.slice(sync)).toMatch(
/^\s*setTargetIndex\(searchTarget\);[^[\]]{0,300}?\}, \[[^\]]*\bsearchTarget\b[^\]]*\]\)/
);
// …and the selector reads the target rather than holding its own copy of it, which a remount
// would reset while the search really had moved.
expect(
containsIgnoringWhitespace(source, SELECTOR_VALUE_CLAMP),
`AutocompleteSearch: selector value clamp not found — expected ${SELECTOR_VALUE_CLAMP}`
).toBe(true);
expect(source).toContain('data={enabledTargets}');
// …and it holds NO uncontrolled copy of it. INVARIANT GUARD — green at the PR base too. The
// PROP, not one spelling of its argument: the previous form named `defaultValue={searchTarget}`
// and left `defaultValue={targetIndex}` unguarded, which was MEASURED — both dropdowns take a
// typechecking `defaultValue` revert with the round-1 suite fully green. Scoped to the
// selector's own source because the text input further down legitimately passes
// `defaultValue={query}`.
expect(selectorSource(source)).not.toMatch(/\bdefaultValue=/);
// INVARIANT GUARD, not regression coverage: this prop predates the PR here. It is load-bearing
// all the same — this change handler casts away the `null` a deselect produces, and unlike the
// sibling it has no fallback, so `searchIndexMap[null]` would reach the provider as an
// undefined index. It is the unguarded copy that a "these two selectors duplicate props"
// tidy-up would delete.
expect(source).toContain('allowDeselect={false}');
});
it('AutocompleteSearch blurs without emptying the carrier, and clears it on navigation', () => {
// MEASURED DEFECT, and the reason the carry did nothing on this component: reaching the
// category selector requires blurring the input, and the blur handler was the CLEAR handler —
// so `''` was written through the carrier a moment before every selector-driven switch.
//
// Two halves, and each is wrong without the other. The blur now empties the display only; the
// URL-follow effect empties the carrier, so the text survives a pick from the selector and
// nothing else. Without the second half, text abandoned at a blur reappears — and is searched
// again — the next time navigation moves the target.
const source = stripComments(read('src/components/AutocompleteSearch/AutocompleteSearch.tsx'));
expect(source).toContain('onBlur={handleBlur}');
expect(source).toContain('onClear={handleClear}');
expect(source).not.toContain('onBlur={handleClear}');
// `onClear?.()` in BOTH, and that is not a duplication to tidy away: on mobile `AppHeader`
// passes `onSearchDone`, so it is what closes the search overlay. A refactor that routes the
// blur past it leaves the overlay stuck open.
expect(source).toMatch(
/const handleClear = \(\) => \{\s*setSearch\(''\);\s*onClear\?\.\(\);\s*\};/
);
expect(source).toMatch(
/const handleBlur = \(\) => \{\s*clearDisplayedText\(\);\s*onClear\?\.\(\);\s*\};/
);
// ONE of the three discards, and a SPELLING check: the carrier is emptied when the URL moves
// the target, immediately before the writer that triggers that remount. The other two —
// submit and Escape — are pinned in the test below, on the same terms.
//
// 🔴 What nothing here covers is that the three are ENOUGH, and they are not. `searchTarget`
// collapses every first path segment outside `targetData` to `'models'`, so navigation that
// stays within one section fires none of them and text blurred away then left alone survives
// to the next selector pick. That is a decision, not an omission; observing it needs a
// rendered input and a router, which is the browser tier.
expect(source).toMatch(/carriedSearchText\.current = '';\s*setTargetIndex\(searchTarget\);/);
});
it('AutocompleteSearch discards the carried text on submit and on Escape', () => {
// MEASURED DEFECT at the previous head: the carrier was emptied only when a navigation moved
// `searchTarget`, so typing on `/`, clicking away and opening a model left text in the
// carrier with no affordance to discard it — the input reads empty and
// `clearable={query.length > 0}` removes the clear button — and the next category pick
// resurrected that text AND searched for it.
//
// SPELLING COVERAGE, and it is the only tier available: both paths run through a rendered
// Mantine input. A rename reddens this for a non-defect; re-pin the new spelling rather than
// loosening the check.
const source = stripComments(read('src/components/AutocompleteSearch/AutocompleteSearch.tsx'));
// Discard and blur in ONE function, so the two "done" paths cannot drift apart.
expect(source).toMatch(
/const blurAndDiscardCarriedText = \(\) => \{\s*carriedSearchText\.current = '';\s*blurInput\(\);\s*\};/
);
expect(source).toContain("['Escape', blurAndDiscardCarriedText]");
expect(source).toMatch(
/const handleSubmit = \(\) => \{[\s\S]{0,400}?blurAndDiscardCarriedText\(\);/
);
// 🔴 The complementary half — that the plain blur handler does NOT discard — is not restated
// here. The test above pins `handleBlur`'s body in FULL, which forbids a discard inside it
// more tightly than any check written here could, and pins `onBlur={handleBlur}` so the input
// cannot be rewired to this function instead. One change should redden one test.
});
it('QuickSearchDropdown drives its index selector from the target it is searching', () => {
const source = stripComments(read('src/components/Search/QuickSearchDropdown.tsx'));
expect(
containsIgnoringWhitespace(source, SELECTOR_VALUE_CLAMP),
`QuickSearchDropdown: selector value clamp not found — expected ${SELECTOR_VALUE_CLAMP}`
).toBe(true);
expect(source).toContain('data={enabledTargets}');
// …and it holds NO uncontrolled copy of the target. 🔴 The previous form here was
// `not.toContain('defaultValue={enabledTargets[0]}')`, which DISCRIMINATED NOTHING:
// `enabledTargets` is `{ label, value }[]` while Mantine's `SelectProps['defaultValue']` is
// `string | null`, so that spelling could never have been written. The plausible reverts —
// `defaultValue={fallbackIndex}`, `defaultValue={enabledTargets[0].value}` — typecheck, and
// the first was MEASURED to leave the round-1 suite fully green. Pinned on the PROP now,
// scoped to the selector's own source because the text input further down legitimately passes
// `defaultValue={query}`. INVARIANT GUARD: green at the PR base too.
expect(selectorSource(source)).not.toMatch(/\bdefaultValue=/);
// DELIBERATELY UNCOVERED, said out loud rather than left as a silent omission: the
// `startingIndex ?? supportedIndexes[0] ?? 'models'` fallback. Its INITIAL-value arm is
// unreachable — every caller either passes `startingIndex` or supports `models` first — and
// its deselect arm needs a rendered Mantine `Select` to reach, which is the browser tier.
// So reverting it to a bare `'models'` leaves this suite green. Stated rather than pinned:
// a spelling check here would be an invariant guard wearing a regression guard's title.
});
it('AutocompleteSearch re-runs its refine effect when search availability recovers', () => {
// `searchErrorState` reads a module-level store, so it is the one input to that effect which
// SURVIVES the remount. Missing from the deps, a tree that remounted while search was
// unavailable restores the typed text, returns early, and never refines once the flag clears
// — a populated box over an empty helper query. Pinned structurally because only a render of
// the real component could observe it, and that is the browser tier.
// Comments stripped first: a dependency array can otherwise satisfy a token search with the
// token sitting inside `/* … */`, which is the walk `openingTag` above already guards against.
const source = stripComments(read('src/components/AutocompleteSearch/AutocompleteSearch.tsx'));
const deps = source.match(/\}, \[debouncedSearch, query, indexName[^\]]*\]/);
expect(deps?.[0] ?? '(no refine dependency array matched)').toContain('searchErrorState');
// The other half — that the flag is still PASSED to the predicate, which a dependency array
// cannot see — is pinned by the refine-gate test above, whose full `if (…)` expression
// subsumes it. Deliberately not restated here; one change should redden one test.
});
});
const roots: Root[] = [];
afterEach(async () => {
for (const root of roots.splice(0)) await act(async () => root.unmount());
});
type Harness = {
render: (
indexName: string,
options?: { helperQuery?: string; blocked?: boolean }
) => Promise<void>;
text: () => string;
type: (value: string) => Promise<void>;
/** What the input's blur handler does: empty the display, leave the carrier alone. */
blur: () => Promise<void>;
refinedWith: () => string[];
};
/**
* The arrangement both dropdowns now use, with the search helper modelled by the smallest thing
* that can be wrong about it: per-mount state that a remount resets to its initial value, plus the
* components' own refine effect — the REAL `shouldRefineSearchQuery`, not a restatement of it.
*
* `carried: false` is the negative control: the same tree with the text seeded from the helper's
* query instead, which is what these components did before.
*/
function mount(carried: boolean): Harness {
const container = document.body.appendChild(document.createElement('div'));
const root = createRoot(container);
roots.push(root);
let write: ((value: string) => void) | null = null;
let clearDisplay: (() => void) | null = null;
const refined: string[] = [];
function Child({
carriedRef,
helperQuery,
blocked,
}: {
carriedRef: React.MutableRefObject<string>;
helperQuery: string;
blocked: boolean;
}) {
const viaCarrier = useCarriedSearchText(carriedRef, helperQuery);
const viaState = React.useState(helperQuery);
const [text, setText] = carried ? viaCarrier : viaState;
write = setText;
// The blur path, per arm. Carried: the hook's display-only clear. The negative-control arm has
// no carrier to spare, so its blur is just an empty write — which is also what the CARRIED arm
// did before this was split, and what made the carry inert on `AutocompleteSearch`.
clearDisplay = carried ? viaCarrier[2] : () => viaState[1]('');
// The helper's own query. Per-mount, so a keyed remount hands the child a rebuilt helper
// reporting whatever it was constructed with — `''` in production.
const [refinedQuery, setRefinedQuery] = React.useState(helperQuery);
React.useEffect(() => {
if (!shouldRefineSearchQuery(text, refinedQuery, blocked)) return;
refined.push(text);
setRefinedQuery(text);
}, [text, refinedQuery, blocked]);
return React.createElement('span', null, text);
}
function Parent({
indexName,
helperQuery,
blocked,
}: {
indexName: string;
helperQuery: string;
blocked: boolean;
}) {
const carriedRef = React.useRef('');
return React.createElement(Child, { key: indexName, carriedRef, helperQuery, blocked });
}
return {
render: async (indexName, { helperQuery = '', blocked = false } = {}) => {
await act(async () =>
root.render(React.createElement(Parent, { indexName, helperQuery, blocked }))
);
},
text: () => container.textContent ?? '',
type: async (value) => {
await act(async () => write?.(value));
},
blur: async () => {
await act(async () => clearDisplay?.());
},
refinedWith: () => [...refined],
};
}
describe('useCarriedSearchText', () => {
it('keeps the typed text when the index changes and the provider is rebuilt', async () => {
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
expect(harness.text()).toBe('dreamshaper');
await harness.render('articles_v6');
expect(harness.text()).toBe('dreamshaper');
});
it('pushes that text into the rebuilt helper, so the search runs again on the new index', async () => {
// The point of the seed. A rebuilt helper reports an empty query, so the carried text differs
// from it and the refine effect fires a SECOND time — the search is re-run rather than the
// input merely re-displaying the old text.
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
expect(harness.refinedWith()).toEqual(['dreamshaper']);
await harness.render('articles_v6');
expect(harness.refinedWith()).toEqual(['dreamshaper', 'dreamshaper']);
});
it('survives the blur that reaching the category selector requires', async () => {
// THE PATH THE FEATURE EXISTS FOR, and the one it did not cover. Clicking the selector blurs
// the input first, so the blur happens BEFORE the index switch, every time. A blur that wrote
// `''` through the carrier therefore emptied it a moment before the remount that was supposed
// to restore it, and the whole carry was inert on `AutocompleteSearch`.
//
// The input still empties on blur — that is unchanged, and asserted here — but the carried
// copy is what the remount reads.
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
await harness.blur();
expect(harness.text()).toBe('');
await harness.render('articles_v6');
expect(harness.text()).toBe('dreamshaper');
// …and it is pushed into the rebuilt helper, so the new index is actually searched for it
// rather than the text merely reappearing. The `''` in the middle is the blur reaching the
// helper, which is what empties the results behind a blurred input today.
expect(harness.refinedWith()).toEqual(['dreamshaper', '', 'dreamshaper']);
});
it('NEGATIVE CONTROL — the same sequence with the blur written THROUGH the carrier loses the text', async () => {
// Identical to the test above except for one step: the empty value goes through the ordinary
// setter instead of the display-only clear. That is exactly what the blur handler used to do,
// and it drops the text on the very switch the carry exists for — so the difference the test
// above measures is the split itself, not something the carrier gave you either way. Same
// carrier, same remount, same assertions; one setter apart.
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
await harness.type('');
await harness.render('articles_v6');
expect(harness.text()).toBe('');
});
it('NEGATIVE CONTROL — the same remount drops the text, and refines nothing, without the carrier', async () => {
// Proves the remount in the tests above is real: without it `viaState` would still hold the
// text and this would fail. Read it together with the two tests above — this one shows the
// CHILD was rebuilt, and those show the parent's ref survived that rebuild. Neither claim
// stands alone.
const harness = mount(false);
await harness.render('models_v9');
await harness.type('dreamshaper');
expect(harness.text()).toBe('dreamshaper');
await harness.render('articles_v6');
expect(harness.text()).toBe('');
expect(harness.refinedWith()).toEqual(['dreamshaper']);
});
it('does not refine while search is unavailable, and refines once it recovers', async () => {
// `blocked` stands for `searchErrorState`, which reads a module-level store and therefore
// SURVIVES the remount. It has to be an input the effect can re-run on: a tree that remounted
// while blocked restores the text, refines nothing, and would otherwise sit on a populated
// input over an empty helper query until the next keystroke.
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
await harness.render('articles_v6', { blocked: true });
expect(harness.text()).toBe('dreamshaper');
expect(harness.refinedWith()).toEqual(['dreamshaper']);
await harness.render('articles_v6', { blocked: false });
expect(harness.refinedWith()).toEqual(['dreamshaper', 'dreamshaper']);
});
it('INVARIANT: carried text outranks a non-empty helper query at the hook, not just in the helper', async () => {
// Both non-empty at once cannot happen in production — a rebuilt helper always reports `''` —
// so this pins a property the bug never violated rather than covering a regression. It earns
// its place by being the only thing that can see the hook's own call into
// `seedCarriedSearchText` with its arguments SWAPPED: every other case has one of the two
// empty, which makes the swap indistinguishable from the correct order.
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
await harness.render('articles_v6', { helperQuery: 'restored-from-url' });
expect(harness.text()).toBe('dreamshaper');
});
it('seeds a first mount from the helper query, since nothing has been typed yet', async () => {
const harness = mount(true);
await harness.render('models_v9', { helperQuery: 'restored-from-url' });
expect(harness.text()).toBe('restored-from-url');
});
it('gives each carrier its own text — two search surfaces on one page do not share', async () => {
// A module-scope slot instead of the passed ref would pass every test above while making the
// header search and a dropdown on the same page overwrite each other.
const typedInto = mount(true);
const untouched = mount(true);
await typedInto.render('models_v9');
await untouched.render('models_v9');
await typedInto.type('dreamshaper');
await untouched.render('articles_v6');
expect(untouched.text()).toBe('');
});
it('an emptied input falls back to the helper query on the next mount', async () => {
const harness = mount(true);
await harness.render('models_v9');
await harness.type('dreamshaper');
await harness.type('');
await harness.render('articles_v6', { helperQuery: 'restored-from-url' });
// An empty carrier falls back to the helper query — the first-mount behaviour above. That is
// the documented precedence, pinned here so a change to it is a decision rather than a drift.
expect(harness.text()).toBe('restored-from-url');
});
});
describe('shouldRefineSearchQuery', () => {
it('refines when the typed text differs from the helper query', () => {
expect(shouldRefineSearchQuery('dreamshaper', '')).toBe(true);
});
it('does not refine when the helper already holds that text', () => {
expect(shouldRefineSearchQuery('dreamshaper', 'dreamshaper')).toBe(false);
});
it('does not refine while blocked, however far apart the two are', () => {
expect(shouldRefineSearchQuery('dreamshaper', '', true)).toBe(false);
});
});
describe('seedCarriedSearchText', () => {
it('prefers carried text over the helper query', () => {
expect(seedCarriedSearchText('dreamshaper', 'restored-from-url')).toBe('dreamshaper');
});
it('falls back to the helper query when nothing is carried', () => {
expect(seedCarriedSearchText('', 'restored-from-url')).toBe('restored-from-url');
expect(seedCarriedSearchText(undefined, 'restored-from-url')).toBe('restored-from-url');
});
});
src/components/Search/useCarriedSearchText.ts
+97
View File
@@ -0,0 +1,97 @@
import type { MutableRefObject } from 'react';
import { useCallback, useState } from 'react';
/**
* Carries the text a user has typed across the remount that an index switch causes.
*
* Both dropdown search roots key `<InstantSearch>` on the resolved index name, the way
* `SearchLayout` already does. That key is what keeps a search from firing with the PREVIOUS
* index's parameters: react-instantsearch-core calls `helper.setIndex(indexName).search()` in its
* RENDER body, and `<InstantSearch>` renders before the children that own the query parameters, so
* without the key a target switch searches the new index with the old target's `filters`. Keying it
* builds a fresh helper instead, and the children mount their parameters onto it before it searches.
*
* The cost of the key is that the typed text lives INSIDE that subtree, so a remount would wipe it.
* `carriedRef` is owned by the component that renders `<InstantSearch>` — above the keyed boundary,
* so it survives — and this hook seeds the remounted input from it and keeps it written.
*
* `refinedQuery` is the search helper's own query. It is only the seed on a FIRST mount: a freshly
* built helper reports `''`, so a remount that carries text seeds a value that differs from it, and
* that difference is what makes each component's existing "push the text into the helper" effect
* fire again. That is what re-RUNS the search on the new index rather than only re-displaying the
* text.
*
* @returns three things: the current text; a setter that writes the carrier as well as the state;
* and a display-only clear that empties the visible text and LEAVES the carrier alone.
*
* Every write of a value goes through the setter — a bare `setState` would leave the carrier
* holding stale text, which the next remount would restore over the newer value.
*
* The display-only clear is the deliberate exception, and it exists for one caller: the blur
* handler on `AutocompleteSearch`'s input. Clicking the category selector blurs that input, so a
* blur that wrote `''` through the setter would empty the carrier a moment BEFORE the switch it
* is meant to survive — which is what made the carry inert on that component. A blur is the
* browser moving focus, not the user asking to discard what they typed; an explicit clear (the
* input's clear button) still goes through the setter and does discard it.
*
* The cost is a window where the input reads empty while the carrier still holds text, so the
* NEXT remount re-seeds text the user last saw cleared. This hook cannot bound that window —
* only the owner of `carriedRef` can, by emptying the ref. `AutocompleteSearch` does so on three
* paths: a submitted search, an Escape, and a navigation that CHANGES the section the search
* follows. 🔴 That is a narrowing, not a closure, and the gap is reachable: a navigation which
* leaves that section unchanged fires none of them, so text blurred away and then left alone
* survives until the next remount re-seeds it. Deliberate — the alternative is discarding on
* every blur, which is what made the carry inert.
*/
export function useCarriedSearchText(
carriedRef: MutableRefObject<string>,
refinedQuery: string
): [string, (value: string) => void, () => void] {
const [text, setText] = useState(() => seedCarriedSearchText(carriedRef.current, refinedQuery));
const write = useCallback(
(value: string) => {
carriedRef.current = value;
setText(value);
},
[carriedRef]
);
const clearDisplayedText = useCallback(() => setText(''), []);
return [text, write, clearDisplayedText];
}
/**
* What a mounting input starts with. Carried text wins; an empty carrier falls back to the helper's
* own query, which is the behaviour a first mount had before the carrier existed.
*/
export function seedCarriedSearchText(carried: string | undefined, refinedQuery: string): string {
return carried ? carried : refinedQuery;
}
/**
* Whether the mounted tree still owes its text to the search helper — the decision both dropdowns'
* "push the text into the helper" effects make.
*
* This is what turns a remount into a re-RUN of the search rather than a re-display of the text: a
* rebuilt helper reports an empty query, so carried text differs from it and gets pushed.
*
* @param blocked reasons not to refine at all — a hit was picked from the list, or search is
* unavailable. 🔴 A source of `blocked` that OUTLIVES the remount must appear in the calling
* effect's dependency array, or a tree that remounted while blocked restores the typed text,
* returns early, and never refines once the block clears — a populated input over an empty
* helper query. `searchErrorState` is such a source (a module-level store) and is listed.
* A source that is per-mount state is reset by the remount, so leaving it out cannot produce
* THAT failure; `AutocompleteSearch`'s `selectedItem` is one, and is left out. ⚠️ Read narrowly
* — this is not a blessing. Omitting a per-mount source still costs the refine cycle in which
* it is stale: after a hit is picked, the next text change evaluates the guard against the old
* `selectedItem` and skips one refine. Pre-existing there, and not something to reproduce.
*/
export function shouldRefineSearchQuery(
typed: string,
refinedQuery: string,
blocked = false
): boolean {
return !blocked && typed !== refinedQuery;
}