Skip to content

Fixes issues with redux. #315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wilkie merged 2 commits into from
May 26, 2026
Merged

Fixes issues with redux. #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18f5f0c
Fixes issues with redux.
wilkie May 26, 2026
6b69938
Removes optimistic use of state.
wilkie May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@code-dot-org/ml-playground",
"version": "0.0.50",
"version": "0.0.51",
"private": false,
"repository": {
"type": "git",
Expand Down Expand Up @@ -81,7 +81,7 @@
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-papaparse": "^3.8.0",
"react-redux": "^9.2.0",
"react-redux": "^8.1.3",
"redux": "^4.0.5",
"style-loader": "^4.0.0",
"ts-loader": "^9.5.7",
Expand Down
67 changes: 47 additions & 20 deletions src/redux.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,20 @@ export default function rootReducer(
}
if (action.type === SET_IMPORTED_DATA) {
if (state.currentPanel === 'selectDataset') {
state.instructionsKeyCallback!(
action.userUploadedData ? 'uploadedDataset' : 'selectedDataset',
null,
);
// Reducer must stay pure: the consumer-supplied callback dispatches
// into its own redux store, which would interleave React commits
// (and a getState cascade) into this dispatch and trip the
// "getState() while reducer is executing" guard. Defer to a
// microtask so the dispatch fully unwinds before the callback fires.
if (state.instructionsKeyCallback) {
const callback = state.instructionsKeyCallback;
queueMicrotask(() =>
Copy link
Copy Markdown

@sanchitmalhotra126 sanchitmalhotra126 May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! Ideally we wouldn't have a callback in here at all and/or move this to a thunk, but that can happen later

Copy link
Copy Markdown
Contributor Author

@wilkie wilkie May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I don't like a bit of how this is working from a certain standpoint, but the key word there is it is working. When we port it into the actual main repo, we have much more opportunity to clean it up. I'm not sure if ml-playground itself really needs its own redux to do what it is doing anyway.

callback(
action.userUploadedData ? 'uploadedDataset' : 'selectedDataset',
null,
),
);
}
}

return {
Expand Down Expand Up @@ -601,7 +611,13 @@ export default function rootReducer(
state.viewedPanels.push(action.currentPanel);
showedOverlay = true;
}
state.instructionsKeyCallback(action.currentPanel, options);
// Deferred to a microtask — see the comment on the SET_IMPORTED_DATA
// branch above for why the reducer must not synchronously fire a
// consumer callback that dispatches into another store.
const callback = state.instructionsKeyCallback;
const callbackAction = action.currentPanel;
const callbackOptions = options;
queueMicrotask(() => callback(callbackAction, callbackOptions));
}

if (action.currentPanel === 'dataDisplayLabel') {
Expand Down Expand Up @@ -670,24 +686,32 @@ export default function rootReducer(
} else if (state.currentColumn === action.currentColumn) {
// If column is selected, then deselect.
if (state.currentPanel === 'dataDisplayFeatures') {
state.instructionsKeyCallback!('dataDisplayFeatures', null);
// Deferred — see SET_IMPORTED_DATA comment.
if (state.instructionsKeyCallback) {
const callback = state.instructionsKeyCallback;
queueMicrotask(() => callback('dataDisplayFeatures', null));
}
}
return {
...state,
currentColumn: undefined,
};
} else {
if (state.currentPanel === 'dataDisplayFeatures') {
if (
state.columnsByDataType[action.currentColumn] ===
ColumnTypes.NUMERICAL
) {
state.instructionsKeyCallback!('selectedFeatureNumerical', null);
} else if (
state.columnsByDataType[action.currentColumn] ===
ColumnTypes.CATEGORICAL
) {
state.instructionsKeyCallback!('selectedFeatureCategorical', null);
// Deferred — see SET_IMPORTED_DATA comment.
if (state.instructionsKeyCallback) {
const callback = state.instructionsKeyCallback;
if (
state.columnsByDataType[action.currentColumn] ===
ColumnTypes.NUMERICAL
) {
queueMicrotask(() => callback('selectedFeatureNumerical', null));
} else if (
state.columnsByDataType[action.currentColumn] ===
ColumnTypes.CATEGORICAL
) {
queueMicrotask(() => callback('selectedFeatureCategorical', null));
}
}
}

Expand Down Expand Up @@ -731,10 +755,13 @@ export default function rootReducer(
};
}
if (action.type === SET_SHOW_RESULTS_DETAILS) {
state.instructionsKeyCallback!(
action.show ? 'resultsDetails' : 'results',
null,
);
// Deferred — see SET_IMPORTED_DATA comment.
if (state.instructionsKeyCallback) {
const callback = state.instructionsKeyCallback;
queueMicrotask(() =>
callback(action.show ? 'resultsDetails' : 'results', null),
);
}
return {
...state,
showResultsDetails: action.show,
Expand Down
8 changes: 8 additions & 0 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ const externalConfig = {
react: 'react',
'react-dom': 'react-dom',
'react/jsx-runtime': 'react/jsx-runtime',
// Externalize redux + react-redux so the consumer's instances are
// used at runtime. Bundling our own copies creates two react-redux
// instances on the page (one ours, one the consumer's) sharing the
// same React reconciler — their subscription notification chains
// interleave and trip redux's "getState() while reducer is
// executing" guard. One instance per page eliminates the race.
redux: 'redux',
'react-redux': 'react-redux',
},
};

Expand Down
47 changes: 35 additions & 12 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,11 @@
"@babel/plugin-transform-modules-commonjs" "^7.27.1"
"@babel/plugin-transform-typescript" "^7.28.5"

"@babel/runtime@^7.12.1":
version "7.29.7"
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.29.7.tgz#12022450c45a4da6d8d8287b18a4ff2ddb23f768"
integrity sha512-Nq8OhGWiZIZGV6hLHoyAKLLcJihP/xFeBMGJoUrxTX2psI8dCifzLhZISFb+VWS3wFMRDmCGw5R+dOySCqPLhw==

"@babel/template@^7.28.6", "@babel/template@^7.3.3":
version "7.28.6"
resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.28.6.tgz#0e7e56ecedb78aeef66ce7972b082fce76a23e57"
Expand Down Expand Up @@ -1821,6 +1826,13 @@
dependencies:
"@types/node" "*"

"@types/hoist-non-react-statics@^3.3.1":
version "3.3.7"
resolved "https://registry.yarnpkg.com/@types/hoist-non-react-statics/-/hoist-non-react-statics-3.3.7.tgz#306e3a3a73828522efa1341159da4846e7573a6c"
integrity sha512-PQTyIulDkIDro8P+IHbKCsw7U2xxBYflVzW/FgWdCAePD9xGSidgA76/GeJ6lBKoblyhf9pBY763gbrN+1dI8g==
dependencies:
hoist-non-react-statics "^3.3.0"

"@types/http-errors@*":
version "2.0.5"
resolved "https://registry.yarnpkg.com/@types/http-errors/-/http-errors-2.0.5.tgz#5b749ab2b16ba113423feb1a64a95dcd30398472"
Expand Down Expand Up @@ -1952,10 +1964,10 @@
resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-2.0.3.tgz#6209321eb2c1712a7e7466422b8cb1fc0d9dd5d8"
integrity sha512-9aEbYZ3TbYMznPdcdr3SmIrLXwC/AKZXQeCf9Pgao5CKb8CyHuEX5jzWPTkvregvhRJHcpRO6BFoGW9ycaOkYw==

"@types/use-sync-external-store@^0.0.6":
version "0.0.6"
resolved "https://registry.yarnpkg.com/@types/use-sync-external-store/-/use-sync-external-store-0.0.6.tgz#60be8d21baab8c305132eb9cb912ed497852aadc"
integrity sha512-zFDAD+tlpf2r4asuHEj0XH6pY6i0g5NeAHPn+15wk3BV6JA69eERFXC1gyGThDkVa1zCyKr5jox1+2LbV/AMLg==
"@types/use-sync-external-store@^0.0.3":
version "0.0.3"
resolved "https://registry.yarnpkg.com/@types/use-sync-external-store/-/use-sync-external-store-0.0.3.tgz#b6725d5f4af24ace33b36fafd295136e75509f43"
integrity sha512-EwmlvuaxPNej9+T4v5AuBPJa2x2UOJVdjCtDHgcDqitUeOtjnJKJ+apYjVcAoBEMjKW1VVFGZLUb5+qqa09XFA==

"@types/ws@^8.5.10":
version "8.18.1"
Expand Down Expand Up @@ -4241,6 +4253,13 @@ hermes-parser@^0.25.1:
dependencies:
hermes-estree "0.25.1"

hoist-non-react-statics@^3.3.0, hoist-non-react-statics@^3.3.2:
version "3.3.2"
resolved "https://registry.yarnpkg.com/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz#ece0acaf71d62c2969c2ec59feff42a4b1a85b45"
integrity sha512-/gGivxi8JPKWNm/W0jSmzcMPpfpPLc3dY/6GxhX2hQ9iGj3aDfklV4ET7NjKpSinLpJ5vafa9iiGIEZg10SfBw==
dependencies:
react-is "^16.7.0"

hoopy@^0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/hoopy/-/hoopy-0.1.4.tgz#609207d661100033a9a9402ad3dea677381c1b1d"
Expand Down Expand Up @@ -6125,7 +6144,7 @@ react-dom@^18.3.1:
loose-envify "^1.1.0"
scheduler "^0.23.2"

react-is@^16.13.1, react-is@^16.8.1:
react-is@^16.13.1, react-is@^16.7.0, react-is@^16.8.1:
version "16.13.1"
resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4"
integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==
Expand All @@ -6143,13 +6162,17 @@ react-papaparse@^3.8.0:
"@types/papaparse" "^5.0.4"
papaparse "^5.2.0"

react-redux@^9.2.0:
version "9.3.0"
resolved "https://registry.yarnpkg.com/react-redux/-/react-redux-9.3.0.tgz#a30113bb6d95c0a715d54dda4308d450fca6ce09"
integrity sha512-KQopgqFo/p/fgmAs5qz6p5RWaNAzq40WAu7fJIXnQpYxFPbJYtsJPWvGeF2rOBaY/kEuV77AVsX8TsQzKm+A/g==
react-redux@^8.1.3:
version "8.1.3"
resolved "https://registry.yarnpkg.com/react-redux/-/react-redux-8.1.3.tgz#4fdc0462d0acb59af29a13c27ffef6f49ab4df46"
integrity sha512-n0ZrutD7DaX/j9VscF+uTALI3oUPa/pO4Z3soOBIjuRn/FzVu6aehhysxZCLi6y7duMf52WNZGMl7CtuK5EnRw==
dependencies:
"@types/use-sync-external-store" "^0.0.6"
use-sync-external-store "^1.4.0"
"@babel/runtime" "^7.12.1"
"@types/hoist-non-react-statics" "^3.3.1"
"@types/use-sync-external-store" "^0.0.3"
hoist-non-react-statics "^3.3.2"
react-is "^18.0.0"
use-sync-external-store "^1.0.0"

react@^18.3.1:
version "18.3.1"
Expand Down Expand Up @@ -7231,7 +7254,7 @@ uri-js@^4.2.2:
dependencies:
punycode "^2.1.0"

use-sync-external-store@^1.4.0:
use-sync-external-store@^1.0.0:
version "1.6.0"
resolved "https://registry.yarnpkg.com/use-sync-external-store/-/use-sync-external-store-1.6.0.tgz#b174bfa65cb2b526732d9f2ac0a408027876f32d"
integrity sha512-Pp6GSwGP/NrPIrxVFAIkOQeyw8lFenOHijQWkUTrDvrF4ALqylP2C/KCkeS9dpUM3KvYRQhna5vt7IL95+ZQ9w==
Expand Down
Loading