rusackas commented on code in PR #42602:
URL: https://github.com/apache/superset/pull/42602#discussion_r3693855770
##########
superset-frontend/packages/superset-core/src/theme/Theme.tsx:
##########
@@ -214,9 +230,42 @@ export class Theme {
emotionCache: createCache({ key: 'superset' }),
});
- this.updateProviders = (theme, antdConfig, emotionCache) => {
- setThemeState({ theme, antdConfig, emotionCache });
- };
+ // Register (and, on unmount, deregister) this provider instance's own
+ // listener rather than assigning a single shared callback on every
+ // render, so every concurrently mounted provider for this Theme
+ // instance receives updates, not just the last one to render.
+ //
+ // Use useLayoutEffect (not useEffect) so registration happens in the
+ // same commit phase as any layout effect elsewhere that might call
+ // setConfig/toggleDarkMode on this instance during mount (e.g. the
+ // docs site's dark-mode sync in StorybookWrapper.jsx, which reads the
+ // toggle and pushes it onto the singleton via a layout effect of its
+ // own). Layout effects run bottom-up, so a listener registered here
+ // (this component is nested inside that caller) is guaranteed to be
+ // in place before an ancestor's layout effect can fire and notify it.
+ // If this were a passive effect instead, an ancestor's layout effect
+ // could call toggleDarkMode before this listener exists, dropping that
+ // notification, and the provider would render stale until a later
+ // toggle.
+ // eslint-disable-next-line react-hooks/rules-of-hooks
+ React.useLayoutEffect(() => {
Review Comment:
Good call, added a test for exactly that: an ancestor layout effect toggling
dark mode during the initial commit, before anything else settles. It
reproduces the drop if registration ever reverts back to `useEffect`, since the
toggle would fire before any listener's registered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
