Skip to content

ailab: convert Redux connect() to typed hooks#73385

Open
sanchitmalhotra126 wants to merge 2 commits into
stagingfrom
sanchit/ailab-redux-updates
Open

ailab: convert Redux connect() to typed hooks#73385
sanchitmalhotra126 wants to merge 2 commits into
stagingfrom
sanchit/ailab-redux-updates

Conversation

@sanchitmalhotra126

@sanchitmalhotra126 sanchitmalhotra126 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Modernizes the Redux usage in the migrated AI Lab package (frontend/packages/labs/ailab/) by replacing connect() with typed react-redux hooks, and clears the useSelector stability warnings that the conversion surfaced. Behavior is unchanged — this is a refactor.

What changed

connect() → hooks. All 24 UIComponents plus App now use typed useAppSelector / useAppDispatch instead of connect() + mapStateToProps / mapDispatchToProps. A new src/hooks.ts is the single source of the typed hooks (useAppSelector: TypedUseSelectorHook<RootState>), so a mistyped state path or prop shape now fails the typecheck. The container/presentational split in Statement is preserved (UnconnectedStatement for Results' explicit-prop usage; a hooks container as the default export).

Selector stability warnings. connect's mapStateToProps never ran react-redux's dev stability check; useSelector does, which exposed several selectors that rebuild a fresh object/array each call:

  • getDatasetDetails, getPanelButtons — read the I18n global, so a state-keyed reselect memo would go stale across a locale change (and broke a unit test when tried). Left as plain functions; deduped by value at the call site with an equality fn (shallowEqual for the flat dataset-details object; a prev/next-aware fn for panel buttons).
  • getLabelToSave, getFeaturesToSave, getTableData (results path) — return nested shapes (values[], arrays of rows) that shallowEqual can't dedupe, and also touch I18n transitively. Added a small structural deepEqual to hooks.ts and pass it as the useSelector equality fn at the call sites.
  • getTrainedModelDataToSave — heavy (trainedModel.toJSON()) and only needed on Save, so it's no longer selected reactively. The save payload is composed in index.tsx's startSaveTrainedModel from store.getState(), where the store already lives. Flagged with a TODO to fold into a thunk during the RTK migration.

Net: the dev console is clean of selector warnings, with no change to rendered output or dispatch behavior.

What this is not

Two follow-ups intentionally left out of scope, to keep this a pure connect→hooks refactor:

  • Converting the slice to Redux Toolkit (createSlice / configureStore).
  • Moving side-effects and the stored instructionsKeyCallback out of the reducer (a listenerMiddleware / thunk concern). The save-payload TODO above is the first thread of this.

Links

  • Jira:

Testing story

  • yarn workspace @code-dot-org/ailab run typecheck — clean.
  • yarn workspace @code-dot-org/ailab test — 108 passing (13 files).
  • ./tools/hooks/pre-commit — clean.

The package's automated coverage is pure-logic only (helpers/selectors/redux); there are no component or e2e tests, so the React wiring this change touches (App/index/UIComponents are 0% covered) is guarded primarily by TypeScript and the unchanged selector bodies. The runtime-only stability warnings can't be asserted by the unit suite, so a manual browser pass is warranted before merge:

  1. Load a dataset → train → Save → confirm status transitions and the success/error panel switch (exercises the index.tsx payload move + getTrainedModelDataToSave).
  2. Step prev/next through all panels (exercises getPanelButtons).
  3. Confirm the dev console is free of "returned a different result when called with the same parameters" warnings on the SelectDataset, DataDisplay, GenerateResults, Results, and ModelSummary panels.

Deployment notes

Standard merge-and-deploy. No migrations, flags, or asset changes.

sanchitmalhotra126 and others added 2 commits June 22, 2026 17:59
Replace connect() with typed react-redux hooks (useAppSelector /
useAppDispatch) across App and all UIComponents, adding src/hooks.ts as
the single typed-hook source.

Fix the useSelector "returned a different result when called with the
same parameters" warnings for getPanelButtons and getDatasetDetails.
Both read the I18n global, so a state-keyed reselect memo would go stale
across a locale change; instead they stay plain functions and the warning
is silenced by deduping at the call site with an equality fn
(shallowEqual for the flat getDatasetDetails result, a prev/next-aware
fn for getPanelButtons).

Stop selecting getTrainedModelDataToSave reactively (heavy, deeply
nested, only needed on Save). The save payload is now composed in
index.tsx's startSaveTrainedModel from store.getState(), where the store
already lives, so App passes the callback through untouched. Flagged for
folding into a thunk during the RTK migration.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The connect()->hooks conversion exposed more "returned a different
result when called with the same parameters" warnings: getTableData
(results path) on GenerateResults/DataTable, and getLabelToSave /
getFeaturesToSave on ModelCard. Each rebuilds a fresh nested value
(array of rows, or a column object with a values[] field) per call.

These all read the I18n global transitively (or, for getTableData,
route through accuracy helpers), so a state-keyed reselect memo isn't
the right tool. Dedupe by value at the call site instead, matching the
shallowEqual/panelButtonsEqual approach already used here. shallowEqual
won't do because the values are nested, so add a small structural
deepEqual to hooks.ts and pass it as the useAppSelector equality fn.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sanchitmalhotra126 sanchitmalhotra126 requested review from a team, stephenliang and wilkie June 22, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant