sadpandajoe commented on code in PR #42602:
URL: https://github.com/apache/superset/pull/42602#discussion_r3685938411


##########
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:
   This initial-mount fix can regress without a failing test: all four tests 
toggle only after `render()` has flushed effects, so reverting this 
registration to `useEffect` would still pass and restore the dropped dark-mode 
notification. Could we add an ancestor layout-effect harness that toggles 
during the initial commit and asserts the nested token consumer is dark?



-- 
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]

Reply via email to