Architecture refactor scout: github-32005295866

[github-actions]

cc @algolia/frontend-experiences-web

Architecture Refactor Scout

Run: github-32005295866

Summary

I scouted the whole monorepo with the module-depth rubric, concentrating on the layers where behavior actually lives: the shared connectors in packages/instantsearch.js/src/connectors, the runtime in src/lib, the highlight/snippet helpers, and how React and Vue wrap the same connectors. No CONTEXT.md or docs/adr/ exists. The recurring friction is shallow shared modules that force each caller to re-assemble the same orchestration: the facet connectors (refinementList, menu, hierarchicalMenu) each hand-roll an identical show-more state machine; the "highlighted parts" pipeline is re-derived verbatim in all three flavors because no deep module owns "attribute → parts"; three refinement connectors each re-wrap sendEvent with the same "only emit on refine" guard; and a couple of purely-presentational widgets (e.g. Stats) duplicate a11y/announcement behavior between the legacy Preact components and the React ui/ layer. Each finding below is a small, high-locality PR that pulls scattered orchestration behind one deeper interface.

Candidate Shortlist

candidate-1: Deepen a show-more controller shared by the facet connectors

• Recommendation strength: Strong
• Files:
  • packages/instantsearch.js/src/connectors/refinement-list/connectRefinementList.ts (~lines 240–259, 401–456)
  • packages/instantsearch.js/src/connectors/menu/connectMenu.ts (~lines 185–201, 292–328)
  • packages/instantsearch.js/src/connectors/hierarchical-menu/connectHierarchicalMenu.ts (~lines 218–238, 307, 398–413)
• Problem: All three facet connectors reimplement the identical show-more state machine: a mutable isShowingMore flag, a toggleShowMore placeholder, a cachedToggleShowMore() indirection (which exists purely to hand renderFn a stable function identity across renders — a subtle invariant every connector must remember), a createToggleShowMore(renderOptions, widget) that flips the flag and calls widget.render(), and a getLimit() that switches between limit and showMoreLimit. The interface each connector exposes (isShowingMore, toggleShowMore, canToggleShowMore) is small, but the implementation is copied byte-for-byte, so the invariants (stable identity, when to recreate the closure, how canToggleShowMore is derived from getLimit()) are re-encoded three times and can drift.
• Proposed change: Introduce one deep module that owns the show-more state — it holds isShowingMore, exposes a stable toggleShowMore, a getLimit(), and the derivation of canToggleShowMore — so each connector constructs it once and reads from it instead of re-declaring the machinery. The exact shape (factory vs. small class, how render is injected) is left for the implementation stage.
• Benefits: Locality — the show-more invariant (especially the stable-identity trick) lives in one file with one test instead of being duplicated across three connectors. Leverage — a connector opts into show-more behavior through a tiny interface. Testability — the state machine becomes directly unit-testable rather than only reachable through a full connector render cycle.
• Risks: Must preserve the stable-identity contract exactly (widgets and hooks rely on toggleShowMore keeping the same reference between renders); hierarchicalMenu slices nested data, so the limit application differs slightly and must stay parameterizable. Purely internal — no public interface change.
• Verification: Run the existing connectRefinementList, connectMenu, connectHierarchicalMenu unit suites plus yarn jest common-widgets -t "RefinementList" / "Menu" / "HierarchicalMenu"; assert toggleShowMore identity is stable across renders and that toggling flips getLimit().

Before:

flowchart LR
  RL[connectRefinementList] --> SM1[show-more state copy]
  Menu[connectMenu] --> SM2[show-more state copy]
  HM[connectHierarchicalMenu] --> SM3[show-more state copy]

Loading

After:

flowchart LR
  RL[connectRefinementList] --> Ctrl[show-more controller]
  Menu[connectMenu] --> Ctrl
  HM[connectHierarchicalMenu] --> Ctrl
  Ctrl --> Detail[isShowingMore / getLimit / stable toggle]

Loading

candidate-2: Own the highlight/snippet parts pipeline in one deep util

• Recommendation strength: Strong
• Files:
  • packages/instantsearch.js/src/helpers/components/Highlight.tsx, Snippet.tsx, ReverseHighlight.tsx, ReverseSnippet.tsx (each ~lines 36–52)
  • packages/react-instantsearch/src/widgets/Highlight.tsx, Snippet.tsx, ReverseHighlight.tsx (~lines 30–36)
  • packages/vue-instantsearch/src/util/parseAlgoliaHit.js, packages/vue-instantsearch/src/components/Highlighter.js
  • Underlying utils: packages/instantsearch.js/src/lib/utils/{getHighlightedParts,getHighlightFromSiblings,reverseHighlightedParts,escape-highlight}.ts
• Problem: There is no deep module for "given a hit + attribute, produce highlighted parts." Instead every flavor re-derives the same ordered pipeline by hand: getPropertyByPath(hit._highlightResult | hit._snippetResult, attribute) → normalize to array (toArray in JS, Array.isArray in React) → getHighlightedParts(unescape(value)), with the reverse variants additionally mapping through reverseHighlightedParts/getHighlightFromSiblings. The caller must know which result path to read, that unescape runs before splitting, how to normalize to an array, and (for reverse) the sibling-merge rule. That knowledge is duplicated across instantsearch.js, react-instantsearch, and vue-instantsearch — a leak across three seams that is drift-prone (React already normalizes arrays differently and drops the warning).
• Proposed change: Add one deep util in instantsearch.js that takes the hit, attribute, result kind (highlight vs. snippet), and reverse flag, and returns the finished parts — absorbing path selection, array normalization, unescape ordering, and the reverse-siblings logic. Each flavor helper then calls it and only renders. Exact signature deferred to implementation.
• Benefits: Leverage — four small tag/escape utils collapse behind one call the flavors share, so the normalization rules live once. Locality — a highlighting bug is fixed in one place for all flavors. Testability — the pipeline (including reverse sibling-merge edge cases) becomes the natural test surface instead of being re-tested per flavor.
• Risks: Cross-flavor change touching three published packages; must keep byte-identical output (highlight rendering is user-visible and snapshot-tested). React currently omits the warning and Vue routes through parseAlgoliaHit — behavior parity must be confirmed, not assumed. Best done additively (introduce the util, then migrate callers).
• Verification: yarn jest common-widgets -t "Highlight" / "Snippet", plus the per-flavor Highlight/Snippet component snapshots in each package; diff rendered parts for mixed-entity and reverse cases before/after.

Before:

flowchart LR
  JS[JS helpers] --> P1[re-derived parts pipeline]
  React[React widgets] --> P2[re-derived parts pipeline]
  Vue[Vue components] --> P3[re-derived parts pipeline]

Loading

After:

flowchart LR
  JS[JS helpers] --> Deep[highlight-parts util]
  React[React widgets] --> Deep
  Vue[Vue components] --> Deep
  Deep --> Detail[path / unescape / split / reverse-siblings]

Loading

candidate-3: Factor the refine-only sendEvent guard into one factory

• Recommendation strength: Worth exploring
• Files:
  • packages/instantsearch.js/src/connectors/rating-menu/connectRatingMenu.ts (~lines 45–72)
  • packages/instantsearch.js/src/connectors/numeric-menu/connectNumericMenu.ts (~lines 140–147)
  • packages/instantsearch.js/src/connectors/toggle-refinement/connectToggleRefinement.ts (~lines 43–83)
  • Shared util: packages/instantsearch.js/src/lib/utils/createSendEventForFacet.ts
• Problem: createSendEventForFacet covers generic facets, but ratingMenu, numericMenu, and toggleRefinement each hand-roll their own createSendEvent that wraps the base sendEvent with the same guard — "only emit a click/filter event when the value is becoming refined, not when it is being cleared" (if (!isRefined) sendEvent(...)). The only thing that varies is the isRefined predicate; the wrapping, event-name defaulting, and payload shape are copied. Callers can't tell that this refine-only semantics exists — it's baked separately into each connector, and a new filtering connector must copy it again.
• Proposed change: Extend the shared send-event helper (or add a sibling factory) so a connector supplies just the isRefined predicate and gets the guarded, correctly-defaulted sendEvent back, replacing the three near-identical local createSendEvent functions. Interface details deferred.
• Benefits: Locality — the "only fire on refine" rule and its default event names live in one tested place. Leverage — adding a new refinement connector means passing a predicate, not re-deriving event glue. Testability — the guard is unit-tested once at the util instead of implicitly through three connectors.
• Risks: The three connectors have subtly different argument tuples (e.g. toggleRefinement reads isRefined out of the args array; ratingMenu compares against getRefinedStar()), so the factory must accommodate those without changing emitted payloads; insights output is asserted in tests, so payload parity is critical.
• Verification: The connectRatingMenu / connectNumericMenu / connectToggleRefinement unit suites and any insights middleware tests that assert emitted event shape; confirm no event fires on un-refine and payloads are unchanged.

Before:

flowchart LR
  Rating[connectRatingMenu] --> G1[refine-only guard copy]
  Numeric[connectNumericMenu] --> G2[refine-only guard copy]
  Toggle[connectToggleRefinement] --> G3[refine-only guard copy]

Loading

After:

flowchart LR
  Rating[connectRatingMenu] --> F[sendEvent factory]
  Numeric[connectNumericMenu] --> F
  Toggle[connectToggleRefinement] --> F
  F --> Detail[guard + defaults, predicate injected]

Loading

candidate-4: Move the Stats announcement behavior into the shared UI layer

• Recommendation strength: Worth exploring
• Files:
  • packages/instantsearch.js/src/components/Stats/Stats.tsx (143 lines; ANNOUNCEMENT_DELAY, visuallyHiddenStyle, aria-live announcement effect ~lines 39–135)
  • packages/react-instantsearch/src/ui/Stats.tsx (109 lines; same constants/effect ~lines 38–103)
  • Target: packages/instantsearch-ui-components/src/components/
• Problem: Stats is a purely presentational widget, yet its accessibility behavior is duplicated between the legacy Preact component and the React ui/ copy: the same ANNOUNCEMENT_DELAY = 1400, the same visuallyHiddenStyle, and the same debounced aria-live="polite" announcement effect. The repo has already standardized on instantsearch-ui-components owning shared markup (per CLAUDE.md), so this is a widget that has not yet been migrated — the announcement contract (delay, hidden style, live region) lives in two places and can silently diverge when the a11y spec changes.
• Proposed change: Extract a shared Stats factory (create<Name>Component({ createElement, Fragment })) into instantsearch-ui-components, owning the class names, the visually-hidden announcement region, and the delay, then have both the JS and React layers consume it — the same migration path already used for newer widgets. Translations stay injectable.
• Benefits: Locality — the announcement/a11y behavior is defined once and ripples to every flavor (and instantsearch.css) at once. Leverage — flavors consume a factory instead of re-declaring markup + timing. Testability — the announcement timing/live-region is tested once in the shared package.
• Risks: Timing-dependent effect (debounced announcement) must behave identically under Preact and React; snapshot/markup and class names are user-facing and CSS-coupled, so structure must match exactly. This follows an existing decision rather than introducing a new pattern, but it is a cross-package move.
• Verification: yarn jest common-widgets -t "Stats", the per-flavor Stats snapshots, and a fake-timer test asserting the announcement fires after ANNOUNCEMENT_DELAY with the correct aria-live region.

Before:

flowchart LR
  JS[JS Stats component] --> A1[announcement + hidden style copy]
  React[React ui/Stats] --> A2[announcement + hidden style copy]

Loading

After:

flowchart LR
  JS[JS Stats component] --> Shared[shared Stats factory]
  React[React ui/Stats] --> Shared
  Shared --> Detail[a11y region + delay + classes]

Loading

candidate-5: Consolidate router + stateMapping wiring behind one seam

• Recommendation strength: Speculative
• Files:
  • packages/instantsearch.js/src/middlewares/createRouterMiddleware.ts (~lines 47–126)
  • packages/instantsearch.js/src/lib/routers/history.ts
  • packages/instantsearch.js/src/lib/stateMappings/{simple,singleIndex}.ts
  • Caller: packages/instantsearch.js/src/lib/InstantSearch.ts (~lines 403–406)
• Problem: InstantSearch already collapses routing: true | object into createRouterMiddleware in a few lines, so the default path is fine. The friction appears for a caller who wants a custom setup: they must instantiate historyRouter (which exposes ~16 options, only three commonly used), separately choose or implement a stateMapping (simple vs. singleIndex, and know that configure is filtered inside the mapping), and pass both into the middleware — three modules the caller assembles by hand, with knowledge (configure filtering, single- vs. multi-index mapping) leaking across their seams.
• Proposed change: Provide a single deeper entry that accepts the routing intent (custom router and/or mapping, write delay) and returns a ready middleware with sensible defaults selected internally, so callers stop wiring the router + mapping + middleware trio themselves. Concrete interface deferred; would need a genuine variation to justify the seam.
• Benefits: Leverage — custom routing becomes one call instead of three coordinated constructions. Locality — the "which mapping, filter configure, default write delay" knowledge concentrates behind the seam.
• Risks: Marked speculative because the default path is already deep and the public historyRouter/stateMapping exports are documented extension points — collapsing them risks reducing legitimate flexibility or introducing a wrapper that is itself pass-through. Any change must not alter emitted URLs. Validate that something actually varies across the new seam before committing.
• Verification: The routing middleware and history router unit suites plus the routing e2e specs (tests/e2e); assert identical URLs for default and custom-router configurations before/after.

Before:

flowchart LR
  Caller[Caller] --> Router[historyRouter]
  Caller --> Mapping[stateMapping]
  Caller --> MW[createRouterMiddleware]

Loading

After:

flowchart LR
  Caller[Caller] --> Seam[routing wiring seam]
  Seam --> Router[historyRouter]
  Seam --> Mapping[stateMapping]
  Seam --> MW[createRouterMiddleware]

Loading

Top Recommendation

Implement candidate-1 (show-more controller) first. It is the deepest, most local win: the show-more state machine — including the easy-to-miss stable-function-identity invariant — is duplicated verbatim across three sibling files in a single package, so pulling it behind one small interface has real depth (a lot of behavior and one tricky invariant behind getLimit()/toggleShowMore) with almost no blast radius. It changes no public interface, so migration and compatibility risk is minimal, and it is directly verifiable through the existing connector and common-widgets suites. The PR is small and self-contained, making it the safest way to establish the pattern before tackling the higher-leverage but cross-flavor candidate-2.

Next Step

To trigger implementation of a selected candidate, run:

/implement candidate-1

Replace candidate-1 with the id of the candidate you want to implement (e.g. /implement candidate-2).

Non-Candidates

• Auto-generating the ~31 React connector hooks (react-instantsearch-core/src/connectors/): each hook is a thin one-line useConnector(connectX, props) pass-through, but they are the package's public, individually-typed, tree-shakeable exports. Replacing them with build-time generation is a broad rewrite that trades a real interface for machinery — rejected as too broad and low-depth.
• A universal UiStateCodec / SearchStateBuilder across all connectors: the per-connector getWidgetUiState/getWidgetSearchParameters encodings (range "min:max", pagination offset, refinement cleanup) differ enough that a single codec abstraction risks being a speculative abstraction over genuinely different logic. Rejected per the "don't introduce a seam unless something actually varies cleanly across it" constraint.
• Splitting InstantSearch.ts middleware composition + render scheduling into new managers: tempting given the 924-line file, but the scheduling methods share private state and the middleware precedence rules are subtle; carving out MiddlewareComposer/RenderScheduler is a large, high-risk change to the core lifecycle, not a small module-deepening PR.
• Migrating every legacy instantsearch.js/src/components widget (CurrentRefinements, Breadcrumb, RangeInput, …) to the shared UI layer: this is a real, already-decided ongoing migration, but doing it wholesale is broad churn. Candidate-4 carves off the single smallest, purest instance (Stats) as one reviewable PR instead.