Architecture refactor scout: github-34816709540

cc @algolia/frontend-experiences-web

# Architecture Refactor Scout

Run: `github-34816709540`

## Summary

I inspected the InstantSearch monorepo end to end, concentrating on the layers where behavior is shared: the connectors and runtime in `packages/instantsearch.js` (`src/connectors/`, `src/lib/`, `src/middlewares/`), the shared markup in `packages/instantsearch-ui-components`, and the React/Vue flavor wrappers. No `CONTEXT.md` or `docs/adr/` exists, so evidence is drawn from file paths and caller behavior. The recurring friction is **shallow modules and leaked orchestration**: several places repeat the same ceremony across many call sites (recommend-widget wrappers, insights event builders, Hogan template helpers), and a couple of runtime concerns (insights user-token resolution, router↔initial-UI-state merging) spread their real decision logic across the lifecycle so callers must know ordering and precedence that no single module owns. The strongest candidates deepen a module by hiding that ceremony or that ordering behind a small, testable interface, each within one package and reviewable as a single PR.

## Candidate Shortlist

candidate-1: Deepen the React recommend-widget wrappers behind one factory

- Recommendation strength: `Strong`
- Files:
- `packages/react-instantsearch/src/widgets/TrendingItems.tsx`
- `packages/react-instantsearch/src/widgets/RelatedProducts.tsx`
- `packages/react-instantsearch/src/widgets/FrequentlyBoughtTogether.tsx`
- `packages/react-instantsearch/src/widgets/LookingSimilar.tsx`
- Problem: These four widget wrappers are ~97–101 lines each and are byte-for-byte identical apart from the widget name, the `use*` hook, the `create*Component` factory, and the `$$widgetType` string. A `diff` of `FrequentlyBoughtTogether.tsx` against `RelatedProducts.tsx` shows only naming differences; the `layout`-wrapping block (`TrendingItems.tsx:71-80`) and the `_itemComponent` `useMemo` that injects `sendEvent` (`TrendingItems.tsx:82-88`) are literally the same code in all four. Each wrapper is a shallow module: its interface (a recommend widget) is nearly as complex as its implementation, and the same plumbing knowledge — how to wrap `layout`, when to `useMemo` the item component, how to thread `sendEvent`, how to split UI props from hook props — is copied four times. Adding a fifth recommend widget means copying the ceremony again, and a fix to the `sendEvent`/layout plumbing must be applied in four places.
- Proposed change: Extract the shared wrapper behavior into one deep helper (a factory or shared internal hook) that takes the per-widget specifics — the hook, the UI-component factory, and the `$$widgetType` — and returns a fully wired React widget. The four widget files shrink to a declaration of those specifics. Do not design the exact signature here; the point is to move the repeated plumbing behind a single small interface.
- Benefits: Locality — the layout/`sendEvent`/`useMemo` logic lives in one place instead of four, so a change or bug fix happens once. Leverage — a new recommend widget becomes a short declaration rather than a full copy. Testability — the shared plumbing gets one focused test surface instead of four near-duplicate widget test files.
- Risks: The wrappers have subtle per-widget type differences (e.g. `RelatedProducts` casts `items as Array>` while `FrequentlyBoughtTogether` uses `Hit` generics); the factory must preserve each widget's public prop types exactly to avoid a breaking type change. Snapshot/DOM output must stay identical. Vue has no equivalent recommend components, so this stays a single-package, React-only PR.
- Verification: `yarn jest packages/react-instantsearch/src/widgets/__tests__` for the four widgets; `yarn jest common-widgets` for the recommend widgets; `yarn workspace react-instantsearch type-check` to confirm public prop types are unchanged.

Before:

```mermaid
flowchart LR
App[App code] --> TI[TrendingItems copy]
App --> RP[RelatedProducts copy]
App --> FBT[FrequentlyBoughtTogether copy]
App --> LS[LookingSimilar copy]
TI --> Plumb[layout + sendEvent + useMemo ceremony x4]
RP --> Plumb
FBT --> Plumb
LS --> Plumb
```

After:

```mermaid
flowchart LR
App[App code] --> TI[TrendingItems decl]
App --> RP[RelatedProducts decl]
App --> FBT[FrequentlyBoughtTogether decl]
App --> LS[LookingSimilar decl]
TI --> Factory[Recommend wrapper factory]
RP --> Factory
FBT --> Factory
LS --> Factory
Factory --> Plumb[layout + sendEvent + useMemo hidden once]
```

candidate-2: Consolidate insights user-token resolution into one resolver

- Recommendation strength: `Strong`
- Files:
- `packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts`
- Problem: Resolving which user token to use is spread across the middleware lifecycle rather than owned by one module, and the rubric's "real bugs live in orchestration" pattern applies directly. Tokens are gathered from several sources at different times — `queuedUserToken` read from the insights queue during factory construction, `userTokenBeforeInit` read via callback, `tokenFromSearchParameters` from `initialParameters.userToken` (`createInsightsMiddleware.ts:303`), `insightsInitParams?.userToken` (`:310`), an `anonymousUserToken` fallback (`:284-296`) — and the actual priority decision is an implicit `if/else if` chain inside `started()` (`:314-332`). The queue is read in more than one place, the cookie-saving side effect is wedged into the last `else if` branch (`:325-331`), and the comments (`:300-309`) carry ordering knowledge that the code shape does not enforce. A caller or maintainer must trace the whole lifecycle to answer "what token wins, and why."
- Proposed change: Move token gathering and the priority decision into one internal resolver that takes the known sources and returns the resolved token plus whether a cookie should be persisted. `started()` then calls it once and applies the result. Keep the existing priority order exactly; only relocate and name it. Do not design the resolver's signature here.
- Benefits: Locality — the priority order and cookie rule live in one place instead of being scattered across construction and `started()`. Leverage — the resolved-token contract becomes the small interface the rest of the middleware depends on. Testability — the priority chain becomes directly unit-testable with source inputs, instead of requiring a full middleware harness that mocks the queue and callbacks to observe the outcome.
- Risks: Token resolution feeds live analytics and the first-query state; a behavioral drift (e.g. reordering precedence or changing when the cookie is written) would be a subtle regression. The refactor must be strictly behavior-preserving and rely on the existing insights tests to prove it.
- Verification: `yarn jest packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware` and the shared insights suites; assert precedence cases (init token vs search-param token vs before-init vs queued vs anonymous) and that the anonymous+cookie path still writes exactly once.

Before:

```mermaid
flowchart LR
Started[started lifecycle] --> Q1[read queue at construct]
Started --> Q2[read queue again]
Started --> Chain[implicit if/else priority]
Chain --> Cookie[cookie side effect in branch]
Caller[Maintainer] --> Started
```

After:

```mermaid
flowchart LR
Started[started lifecycle] --> Resolver[Token resolver]
Resolver --> Token[resolved token + cookie decision]
Resolver --> Sources[all sources gathered once]
Caller[Maintainer] --> Resolver
```

candidate-3: Unify the insights sendEvent payload builders

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/lib/utils/createSendEventForHits.ts`
- `packages/instantsearch.js/src/lib/utils/createSendEventForFacet.ts`
- `packages/instantsearch.js/src/connectors/toggle-refinement/connectToggleRefinement.ts`
- Problem: There are three parallel implementations of the same job — turn variadic `sendEvent(...)` arguments into an `InsightsEvent` and hand it to `instantSearchInstance.sendEventToInsights`. Each independently re-derives the same conventions: detect a single-object "custom payload" (`createSendEventForHits.ts:47`, `createSendEventForFacet.ts:36`), split the event string on `:` into type + modifier (`createSendEventForHits.ts:50`, `createSendEventForFacet.ts:33`), default the index from `helper.lastResults?.index || helper.state.index` (`createSendEventForHits.ts:105`, `createSendEventForFacet.ts:48`), validate argument arity, and throw a `__DEV__`-only guidance error. `connectToggleRefinement.ts` reimplements the facet-style check inline rather than reusing the facet builder. Callers (connectors) must know the arg conventions and the divergent payload shapes (`clickedObjectIDsAfterSearch` vs `clickedFilters`), so the argument-parsing knowledge leaks across the seam into every connector's tests.
- Proposed change: Factor the shared parsing/validation/index-defaulting into one internal helper that the hits and facet builders (and the toggle connector) delegate to, leaving each builder to describe only its payload shape. This deepens a single "build an insights event from sendEvent args" module and removes the third, inline copy. Do not design the shared helper's interface here.
- Benefits: Locality — arg parsing, custom-payload detection, the `:` split, and index defaulting live once. Leverage — a new event-emitting connector describes only its payload shape instead of re-deriving the parsing rules. Testability — the shared parsing gets one test surface; the toggle connector stops carrying its own copy of facet logic.
- Risks: The hits and facet payloads genuinely differ (chunking and `queryID` for hits, `isFacetRefined` gating for facets), so the shared piece must be scoped to the parsing/validation that is actually identical and not force a false abstraction over payload construction. Dev-mode error messages are asserted in tests and should be preserved.
- Verification: `yarn jest packages/instantsearch.js/src/lib/utils/__tests__/createSendEventForHits` and `createSendEventForFacet`; `yarn jest packages/instantsearch.js/src/connectors/toggle-refinement`; confirm bind-event/data-attribute output and `__DEV__` error text are unchanged.

Before:

```mermaid
flowchart LR
Hits[connectHits/etc] --> H[createSendEventForHits]
Facets[menu/refinementList] --> F[createSendEventForFacet]
Toggle[connectToggleRefinement] --> Inline[inline copy of facet logic]
H --> Parse1[arg parse + split + index default]
F --> Parse2[arg parse + split + index default]
Inline --> Parse3[arg parse + split + index default]
```

After:

```mermaid
flowchart LR
Hits[connectHits/etc] --> H[hits payload shape]
Facets[menu/refinementList] --> F[facet payload shape]
Toggle[connectToggleRefinement] --> F
H --> Shared[shared arg parse + validate + index default]
F --> Shared
```

candidate-4: Give router and initialUiState merging one owner with explicit precedence

- Recommendation strength: `Worth exploring`
- Files:
- `packages/instantsearch.js/src/middlewares/createRouterMiddleware.ts`
- `packages/instantsearch.js/src/lib/InstantSearch.ts`
- Problem: The initial UI state is assembled in two modules whose contract is only implicit. `InstantSearch.ts` sets `this._initialUiState` from constructor options, then the router middleware's `subscribe()` reads the URL via the `StateMapping`, merges it with `_initialUiState`, and writes the result back (`createRouterMiddleware.ts:105-108`), after which `mainIndex.init()` consumes the merged value. Precedence between the two sources is encoded purely by spread order, and there is a warning about `initialUiState` being "overwritten" that the code does not actually enforce. A caller who wants to understand what the first render sees, or add a third source, must know the read→merge→write sequence lives half in the runtime and half in the middleware and depends on when `subscribe()` runs relative to `init()`.
- Proposed change: Give the "compute the effective initial UI state" step a single owner with an explicit, documented precedence, so the runtime and the router agree on one merge point instead of coordinating through a shared mutable field and spread order. Do not design the owning interface here.
- Benefits: Locality — merge precedence and timing concentrate in one place. Leverage — the effective-initial-state contract becomes the small interface `init()` depends on, rather than an implicitly-mutated field. Testability — precedence can be tested directly, without asserting call order through a mocked router.
- Risks: Routing and SSR hydration are delicate and widely depended on; timing relative to `subscribe()`/`init()` must be preserved exactly, and the existing "overwritten" warning behavior should be kept or made faithful. Higher regression surface than candidates 1–3, hence `Worth exploring` rather than `Strong`.
- Verification: `yarn jest packages/instantsearch.js/src/middlewares/__tests__/createRouterMiddleware` and the routing/URL-sync suites; verify router-state-wins precedence, the overwrite warning, and SSR `initialUiState` hydration behavior.

Before:

```mermaid
flowchart LR
Opts[constructor initialUiState] --> Field[_initialUiState mutable field]
Router[router subscribe] --> Field
Field --> Init[mainIndex.init]
Init --> Precedence[precedence implied by spread order]
```

After:

```mermaid
flowchart LR
Opts[constructor initialUiState] --> Owner[initial-state resolver]
Router[router state] --> Owner
Owner --> Effective[effective initial state, explicit precedence]
Effective --> Init[mainIndex.init]
```

candidate-5: Fold Hogan template-helper JSON parsing into one factory

- Recommendation strength: `Speculative`
- Files:
- `packages/instantsearch.js/src/lib/createHelpers.ts`
- Problem: Five of the six Hogan helpers (`highlight`, `reverseHighlight`, `snippet`, `reverseSnippet`, `insights`) repeat the identical shape: `JSON.parse(options)`, spread into the underlying helper with `hit: this`, and a `try/catch` that throws a near-identical "expects a JSON object" error (`createHelpers.ts:42-125`). The JSON-string protocol and its render-time failure mode are duplicated per helper and leaked to template authors, and each new helper copies the boilerplate again.
- Proposed change: Introduce one internal factory that wraps a helper with the parse + `hit: this` + error-message behavior, so each helper declares only its underlying function and its error text. Do not design the factory's signature here.
- Benefits: Locality — the parse/error protocol lives once. Leverage — adding or changing a helper is a one-line declaration. Testability — the error/parse behavior gets a single test surface.
- Risks: This is a legacy Hogan-templating path with modest depth payoff, and the error strings are asserted in tests, so the win is small and mostly cleanup-adjacent — hence `Speculative`. Only worth doing if it clearly reduces the module's surface without churn.
- Verification: `yarn jest packages/instantsearch.js/src/lib/__tests__/createHelpers`; confirm each helper's success output and the exact `__DEV__` error text are unchanged.

Before:

```mermaid
flowchart LR
Tpl[template authors] --> H1[highlight try/parse/catch]
Tpl --> H2[snippet try/parse/catch]
Tpl --> H3[insights try/parse/catch]
H1 --> Dup[duplicated JSON protocol x5]
H2 --> Dup
H3 --> Dup
```

After:

```mermaid
flowchart LR
Tpl[template authors] --> H1[highlight decl]
Tpl --> H2[snippet decl]
Tpl --> H3[insights decl]
H1 --> Factory[helper factory hides JSON protocol]
H2 --> Factory
H3 --> Factory
```

## Top Recommendation

Implement **candidate-1 (deepen the React recommend-widget wrappers behind one factory)** first. It has the highest locality of the shortlist — the change is confined to four files in a single package, with no cross-flavor ripple since Vue has no recommend components — and the evidence is unambiguous: a `diff` of the widgets shows they differ only in names and strings, with the layout/`sendEvent`/`useMemo` plumbing copied verbatim. The depth gain is real: a factory hides four widgets' worth of wiring behind a small per-widget declaration, giving 4× leverage today and making the next recommend widget nearly free. Because behavior and public prop types can be held constant and verified by the existing per-widget and common-widget tests, the PR is small, low-risk, and easy to review. Candidate-2 is the strongest runtime deepening and a natural follow-up, but it edits live analytics logic and carries more regression risk, so it should come second.

## Next Step

To start implementation of a selected candidate, run:

`/implement candidate-1`

Replace `candidate-1` with the id of the candidate you want to implement (`candidate-1` through `candidate-5`).

## Non-Candidates

- **A generic `createConnector` factory to remove `init`/`render`/`getRenderState` boilerplate across all 30+ connectors.** Tempting as duplication, but it spans every connector at once (a broad rewrite, not one reviewable PR) and the per-connector `getRenderState` differences are structural schema shapes; the deletion test suggests the wrapper is thin but pervasive, making a safe incremental seam hard to introduce. Rejected as too broad for now.
- **Replacing the JS-only Template/Preact component system with shared UI components.** A multi-PR migration touching many widgets and the shared UI package; too large and better handled by the existing `/port-widget` migration path than as a depth refactor.
- **Migrating Vue components off mixins to the Composition API.** Real coupling exists in `mixins/widget.js`, but this is a framework-wide rewrite affecting every Vue component, not a localized deepening, and risks behavioral drift across Vue 2 and Vue 3.
- **Auto-injecting hooks (`useState`/`useEffect`/…) into `create*Component` factories in `instantsearch-ui-components`.** The per-component hook requirements are a legitimate seam, but nothing varies across it today beyond framework identity, so introducing an abstraction would be speculative under the rubric.
- **Adding a public "silent state update" API to `algoliasearch-helper` to replace `overrideStateWithoutTriggeringChangeEvent` callers.** Changes a mature, separately-versioned package for a naming/ergonomics gain; out of scope for a module-depth refactor and against the repo guidance to prefer fixing the connector layer.

First choose one candidate, since the issue presents a shortlist rather than a single scoped change. For the React wrapper candidate, read the four widget files and run the widget Jest suites plus the react-instantsearch type-check; for the insights candidates, start with the named middleware or utility files and their listed tests. Done means the selected behavior remains unchanged while the repeated orchestration has one focused owner.

Written by the indexing model from the issue text.