Skip to content

feat(frontend): enable React StrictMode at root #39893

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

Open
rusackas wants to merge 2 commits into
base: master
Choose a base branch
from
+42 −29
Open

feat(frontend): enable React StrictMode at root #39893

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
893a780
feat(frontend): enable React StrictMode at root
claude May 5, 2026
04dcd88
fix(embedded): move dataMask store.subscribe into an effect
claude May 6, 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
25 changes: 15 additions & 10 deletions superset-frontend/src/embedded/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
import 'src/public-path';

import { lazy, Suspense } from 'react';
import { lazy, StrictMode, Suspense, useEffect } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { BrowserRouter as Router, Route } from 'react-router-dom';
import { Global } from '@emotion/react';
Expand Down Expand Up @@ -68,18 +68,19 @@ const LazyDashboardPage = lazy(

const EmbededLazyDashboardPage = () => {
const uiConfig = useUiConfig();
const emitDataMasks = uiConfig?.emitDataMasks;

// Emit data mask changes to the parent window
if (uiConfig?.emitDataMasks) {
// Emit data mask changes to the parent window. Subscribing inside an effect
// (rather than during render) ensures the unsubscribe runs on unmount,
// including StrictMode's dev-mode double-mount cycle.
useEffect(() => {
if (!emitDataMasks) return undefined;
log('setting up Switchboard event emitter');

let previousDataMask = store.getState().dataMask;

store.subscribe(() => {
const currentState = store.getState();
const currentDataMask = currentState.dataMask;

// Only emit if the dataMask has changed
return store.subscribe(() => {
const currentDataMask = store.getState().dataMask;
if (previousDataMask !== currentDataMask) {
Switchboard.emit('observeDataMask', {
...currentDataMask,
Expand All @@ -88,7 +89,7 @@ const EmbededLazyDashboardPage = () => {
previousDataMask = currentDataMask;
}
});
}
}, [emitDataMasks]);

return <LazyDashboardPage idOrSlug={bootstrapData.embedded!.dashboard_id} />;
};
Expand Down Expand Up @@ -196,7 +197,11 @@ function start() {
if (!root) {
root = createRoot(appMountPoint);
}
root.render(<EmbeddedApp />);
root.render(
<StrictMode>
<EmbeddedApp />
</StrictMode>,
);
Comment on lines +200 to +204
Copy link
Copy Markdown
Contributor

@codeant-ai-for-open-source codeant-ai-for-open-source Bot May 5, 2026

Choose a reason for hiding this comment

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

Suggestion: Wrapping the embedded tree in StrictMode will trigger an extra mount/unmount cycle in development, and this code path includes a Redux store.subscribe side effect inside render (EmbededLazyDashboardPage) without cleanup. That causes duplicate active subscriptions and repeated observeDataMask emissions after startup. Move the subscription logic into an effect with cleanup (or gate StrictMode here until that cleanup is implemented) so StrictMode does not create leaked duplicate listeners. [memory leak]

Severity Level: Major ⚠️ 
- ❌ Embedded dev dashboards emit duplicate observeDataMask events per change.
- ⚠️ Host apps receive duplicated observeDataMask callbacks in development.
- ⚠️ Redux store subscriptions accumulate without cleanup in embedded iframe.
Steps of Reproduction ✅ 
1. In a host app, call `embedDashboard` from `superset-embedded-sdk/src/index.ts` with
`dashboardUiConfig.emitDataMasks: true` (see `UiConfigType.emitDataMasks` at lines 36-40
and `calculateConfig()` adding `+16` for `emitDataMasks` at lines 143-156, then
`dashboardConfigUrlParams` including `uiConfig` at lines 168-170). This generates an
iframe src like `/embedded/<id>?uiConfig=<bitmask>`.

2. The iframe loads the embedded frontend route defined in
`superset-frontend/src/embedded/index.tsx` where `EmbeddedApp` (lines 117-123) renders
`<Route path="/embedded/:uuid/" component={EmbeddedRoute} />`. `EmbeddedRoute` (lines
96-115) wraps children in `EmbeddedContextProviders`, which in turn wraps them in
`EmbeddedUiConfigProvider` (`superset-frontend/src/components/UiConfigContext/index.tsx`
lines 48-66).

3. Inside `EmbeddedUiConfigProvider`, `getUrlParam(URL_PARAMS.uiConfig)` (lines 51-59 of
`UiConfigContext/index.tsx`) decodes the `uiConfig` bitmask and sets `emitDataMasks:
(config & 16) !== 0`. `EmbededLazyDashboardPage` in
`superset-frontend/src/embedded/index.tsx` (lines 69-94) calls `useUiConfig()`, and when
`uiConfig.emitDataMasks` is true, it executes the side-effect block at lines 73-91 that
declares `let previousDataMask = store.getState().dataMask;` and calls `store.subscribe(()
=> { ... Switchboard.emit('observeDataMask', ...) })` at lines 76-88, with no captured
unsubscribe function or cleanup.

4. The embedded app is started when the iframe's `MessageChannel` handshake triggers
`Switchboard.defineMethod('guestToken', ...)` in
`superset-frontend/src/embedded/index.tsx` (lines 259-264), which calls
`setupGuestClient(guestToken)` and then `start()`. In `start()` (same file, lines
179-217), after fetching `/api/v1/me/roles/`, the code creates the React root if needed
and then calls `root.render(<StrictMode><EmbeddedApp /></StrictMode>);` at lines 197-203
(the PR's new code).

5. In development builds, React 18 `StrictMode` intentionally double-invokes function
components on initial mount. As a result, `EmbededLazyDashboardPage`'s function body
(including the `store.subscribe` call at line 78) runs twice, registering two Redux
subscriptions without any corresponding unsubscribe on unmount. On each subsequent
data-mask-related store update, both subscribers execute, each calling
`Switchboard.emit('observeDataMask', ...)` (lines 84-87), so the host's `observeDataMask`
callback registered via `ourPort.defineMethod('observeDataMask', callbackFn)` in
`superset-embedded-sdk/src/index.ts` (lines 34-38 of the second chunk) is invoked multiple
times per state change. Because no cleanup exists, these subscriptions persist for the
lifetime of the page, creating leaked duplicate listeners specifically exposed by the new
StrictMode wrapper.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** superset-frontend/src/embedded/index.tsx
**Line:** 199:203
**Comment:**
	*Memory Leak: Wrapping the embedded tree in `StrictMode` will trigger an extra mount/unmount cycle in development, and this code path includes a Redux `store.subscribe` side effect inside render (`EmbededLazyDashboardPage`) without cleanup. That causes duplicate active subscriptions and repeated `observeDataMask` emissions after startup. Move the subscription logic into an effect with cleanup (or gate StrictMode here until that cleanup is implemented) so StrictMode does not create leaked duplicate listeners.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Copy link
Copy Markdown
Member Author

@rusackas rusackas May 6, 2026

Choose a reason for hiding this comment

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

Good catch — fix pushed in 04dcd88. The store.subscribe was inside the render body without cleanup, which (independently of StrictMode) leaked a new subscription on every re-render. Moved it into a useEffect keyed on emitDataMasks, returning the Redux unsubscribe so it tears down on unmount and on the StrictMode dev-mode double-mount. The pre-existing latent leak is now fixed too.

},
err => {
// something is most likely wrong with the guest token; reset the guard
Expand Down
7 changes: 6 additions & 1 deletion superset-frontend/src/views/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import 'src/public-path';

import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import { logging } from '@apache-superset/core/utils';
import initPreamble from 'src/preamble';
Expand All @@ -31,7 +32,11 @@ if (appMountPoint) {
await initPreamble();
} finally {
const { default: App } = await import(/* webpackMode: "eager" */ './App');
root.render(<App />);
root.render(
<StrictMode>
<App />
</StrictMode>,
);
}
})().catch(err => {
logging.error('Unhandled error during app initialization', err);
Expand Down
39 changes: 21 additions & 18 deletions superset-frontend/src/views/menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import 'src/public-path';

// Menu App. Used in views that do not already include the Menu component in the layout.
// eg, backend rendered views
import { StrictMode } from 'react';
import { Provider } from 'react-redux';
import { createRoot } from 'react-dom/client';
import { BrowserRouter } from 'react-router-dom';
Expand All @@ -45,24 +46,26 @@ const emotionCache = createCache({
});

const app = (
<CacheProvider value={emotionCache}>
<ThemeProvider theme={theme}>
<Provider store={store}>
<BrowserRouter>
<QueryParamProvider
adapter={ReactRouter5Adapter}
options={{
searchStringToObject: querystring.parse,
objectToSearchString: (object: Record<string, any>) =>
querystring.stringify(object, { encode: false }),
}}
>
<Menu data={menu} />
</QueryParamProvider>
</BrowserRouter>
</Provider>
</ThemeProvider>
</CacheProvider>
<StrictMode>
<CacheProvider value={emotionCache}>
<ThemeProvider theme={theme}>
<Provider store={store}>
<BrowserRouter>
<QueryParamProvider
adapter={ReactRouter5Adapter}
options={{
searchStringToObject: querystring.parse,
objectToSearchString: (object: Record<string, any>) =>
querystring.stringify(object, { encode: false }),
}}
>
<Menu data={menu} />
</QueryParamProvider>
</BrowserRouter>
</Provider>
</ThemeProvider>
</CacheProvider>
</StrictMode>
);

const menuMountPoint = document.getElementById('app-menu');
Expand Down
Loading