ailab: convert Redux connect() to typed hooks#73385
Open
sanchitmalhotra126 wants to merge 2 commits into
Open
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Modernizes the Redux usage in the migrated AI Lab package (
frontend/packages/labs/ailab/) by replacingconnect()with typed react-redux hooks, and clears theuseSelectorstability warnings that the conversion surfaced. Behavior is unchanged — this is a refactor.What changed
connect()→ hooks. All 24 UIComponents plusAppnow use typeduseAppSelector/useAppDispatchinstead ofconnect()+mapStateToProps/mapDispatchToProps. A newsrc/hooks.tsis 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 inStatementis preserved (UnconnectedStatementforResults' explicit-prop usage; a hooks container as the default export).Selector stability warnings.
connect'smapStateToPropsnever ran react-redux's dev stability check;useSelectordoes, which exposed several selectors that rebuild a fresh object/array each call:getDatasetDetails,getPanelButtons— read the I18n global, so a state-keyedreselectmemo 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 (shallowEqualfor 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) thatshallowEqualcan't dedupe, and also touch I18n transitively. Added a small structuraldeepEqualtohooks.tsand pass it as theuseSelectorequality 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 inindex.tsx'sstartSaveTrainedModelfromstore.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:
createSlice/configureStore).instructionsKeyCallbackout of the reducer (alistenerMiddleware/ thunk concern). The save-payload TODO above is the first thread of this.Links
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:
index.tsxpayload move +getTrainedModelDataToSave).getPanelButtons).Deployment notes
Standard merge-and-deploy. No migrations, flags, or asset changes.