test(theme): add regression harness for initial-mount dark-mode toggle race

Covers the scenario from review: an ancestor layout effect (like the
docs site's ThemeSync) toggling dark mode during the initial commit,
before any passive effect would run. Fails if provider listener
registration ever regresses from useLayoutEffect back to useEffect.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

docs/src/components/StorybookWrapper.jsx

@@ -68,6 +68,8 @@ function getProviders() {
     const { themeObject } = require('@apache-superset/core/theme');
     // eslint-disable-next-line @typescript-eslint/no-require-imports
     const { App, ConfigProvider } = require('antd');
+    // eslint-disable-next-line @typescript-eslint/no-require-imports
+    const { useColorMode } = require('@docusaurus/theme-common');
 
     // Configure Ant Design to render portals (tooltips, dropdowns, etc.)
     // inside the closest .storybook-example container instead of document.body
@@ -78,15 +80,39 @@ function getProviders() {
       return container || document.body;
     };
 
+    // `themeObject` is a module-level singleton (superset-core/src/theme
+    // index.tsx: `Theme.fromConfig()`), created once with no dark/light
+    // config, so SupersetThemeProvider always rendered whatever that default
+    // algorithm was -- it had no way to know about Docusaurus's theme toggle.
+    // Docusaurus tracks the toggle in React context (useColorMode), so
+    // mirror it onto the singleton via the toggleDarkMode() method Theme
+    // already exposes for exactly this purpose.
+    //
+    // Use useLayoutEffect (not useEffect) so the sync runs before the
+    // browser paints. This component only ever mounts client-side (it's
+    // built inside a BrowserOnly callback), so there's no SSR mismatch
+    // concern -- and running synchronously before paint avoids a brief
+    // flash of the singleton's previous palette when a page loads directly
+    // in dark mode or the toggle fires during route navigation.
+    function ThemeSync({ children }) {
+      const { colorMode } = useColorMode();
+      React.useLayoutEffect(() => {
+        themeObject.toggleDarkMode(colorMode === 'dark');
+      }, [colorMode]);
+      return children;
+    }
+
     SupersetProviders = ({ children }) => (
-      <themeObject.SupersetThemeProvider>
-        <ConfigProvider
-          getPopupContainer={getPopupContainer}
-          getTargetContainer={() => document.body}
-        >
-          <App>{children}</App>
-        </ConfigProvider>
-      </themeObject.SupersetThemeProvider>
+      <ThemeSync>
+        <themeObject.SupersetThemeProvider>
+          <ConfigProvider
+            getPopupContainer={getPopupContainer}
+            getTargetContainer={() => document.body}
+          >
+            <App>{children}</App>
+          </ConfigProvider>
+        </themeObject.SupersetThemeProvider>
+      </ThemeSync>
     );
     return SupersetProviders;
   } catch (error) {
@@ -133,7 +159,7 @@ function LoadingPlaceholder() {
   return (
     <div
       style={{
-        border: '1px solid #e8e8e8',
+        border: '1px solid var(--ifm-color-emphasis-300)',
         borderRadius: '4px',
         padding: '20px',
         marginBottom: '20px',
@@ -141,7 +167,7 @@ function LoadingPlaceholder() {
         display: 'flex',
         alignItems: 'center',
         justifyContent: 'center',
-        color: '#999',
+        color: 'var(--ifm-color-emphasis-600)',
       }}
     >
       Loading component...
@@ -162,7 +188,7 @@ export function StoryExample({ component, props = {} }) {
             <div
               className="storybook-example"
               style={{
-                border: '1px solid #e8e8e8',
+                border: '1px solid var(--ifm-color-emphasis-300)',
                 borderRadius: '4px',
                 padding: '20px',
                 marginBottom: '20px',
@@ -172,7 +198,7 @@ export function StoryExample({ component, props = {} }) {
               {Component ? (
                 <Component {...restProps}>{children}</Component>
               ) : (
-                <div style={{ color: '#999' }}>
+                <div style={{ color: 'var(--ifm-color-emphasis-600)' }}>
                   Component &quot;{String(component)}&quot; not found
                 </div>
               )}
@@ -341,7 +367,7 @@ function StoryWithControlsInner({ component, renderComponent, props, controls, s
         <div
           className="storybook-example"
           style={{
-            border: '1px solid #e8e8e8',
+            border: '1px solid var(--ifm-color-emphasis-300)',
             borderRadius: '4px',
             padding: '20px',
             marginBottom: '20px',
@@ -359,7 +385,7 @@ function StoryWithControlsInner({ component, renderComponent, props, controls, s
               <Component {...filteredProps} {...triggerProps}>{children}</Component>
             </>
           ) : (
-            <div style={{ color: '#999' }}>
+            <div style={{ color: 'var(--ifm-color-emphasis-600)' }}>
               Component &quot;{String(componentToRender)}&quot; not found
             </div>
           )}
@@ -369,7 +395,7 @@ function StoryWithControlsInner({ component, renderComponent, props, controls, s
           <div
             className="storybook-controls"
             style={{
-              border: '1px solid #e8e8e8',
+              border: '1px solid var(--ifm-color-emphasis-300)',
               borderRadius: '4px',
               padding: '20px',
               marginBottom: '20px',
@@ -483,7 +509,7 @@ function ComponentGalleryInner({ component, sizes, styles, sizeProp, styleProp }
 
   if (!Component) {
     return (
-      <div style={{ color: '#999' }}>
+      <div style={{ color: 'var(--ifm-color-emphasis-600)' }}>
         Component &quot;{String(component)}&quot; not found
       </div>
     );
@@ -494,7 +520,14 @@ function ComponentGalleryInner({ component, sizes, styles, sizeProp, styleProp }
       <div className="component-gallery">
         {sizes.map(size => (
           <div key={size} style={{ marginBottom: 40 }}>
-            <h4 style={{ marginBottom: 16, color: '#666' }}>{size}</h4>
+            <h4
+              style={{
+                marginBottom: 16,
+                color: 'var(--ifm-color-emphasis-700)',
+              }}
+            >
+              {size}
+            </h4>
             <div style={{ display: 'flex', flexWrap: 'wrap', gap: '12px', alignItems: 'center' }}>
               {styles.map(style => (
                 <Component

superset-frontend/packages/superset-core/src/theme/SupersetThemeProvider.test.tsx

@@ -0,0 +1,205 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useLayoutEffect, type ReactNode } from 'react';
+import { render, screen, act } from '@testing-library/react';
+import { theme as antdThemeImport } from 'antd';
+import { Theme } from './Theme';
+
+// SupersetThemeProvider stores theme state via React.useState, then
+// registers a listener (in a useEffect, on mount) that calls setThemeState
+// whenever a *later* call to setConfig/toggleDarkMode runs on the same
+// Theme instance. Every provider currently mounted from that instance
+// listens independently, so toggling the instance updates all of them, not
+// just the most recently rendered one. Consumers
+// (docs/src/components/StorybookWrapper.jsx in particular) rely on this:
+// they call toggleDarkMode() from outside, on one or more already-mounted
+// providers sharing a single Theme instance, expecting it to propagate to
+// all of them.
+//
+// The probe below reads the theme via antd's theme.useToken() -- the same
+// context-consumption path every real antd component (Button, Input, ...)
+// uses internally -- rather than reading themeObject.theme directly off the
+// singleton. That distinction matters: React bails out of re-rendering a
+// child whose element reference didn't change (the common "static children
+// prop" case, true here since <Probe /> is passed once and never
+// recreated), UNLESS that child consumes a React Context whose value
+// changed, which bypasses the bail-out. A probe reading the plain object
+// directly would misleadingly appear "not updated" even though every real
+// themed component downstream re-renders correctly.
+function makeProbe() {
+  let renderCount = 0;
+  let lastColorBgBase: string | undefined;
+  function Probe() {
+    const { token } = antdThemeImport.useToken();
+    renderCount += 1;
+    lastColorBgBase = token.colorBgBase;
+    return <div data-test="probe" />;
+  }
+  return {
+    Probe,
+    getRenderCount: () => renderCount,
+    getLastColorBgBase: () => lastColorBgBase,
+  };
+}
+
+test('an already-mounted SupersetThemeProvider re-renders context-consuming children when toggleDarkMode is called on the same instance', () => {
+  const themeObject = Theme.fromConfig();
+  const { Probe, getRenderCount, getLastColorBgBase } = makeProbe();
+
+  render(
+    <themeObject.SupersetThemeProvider>
+      <Probe />
+    </themeObject.SupersetThemeProvider>,
+  );
+
+  expect(screen.getByTestId('probe')).toBeTruthy();
+  const rendersBefore = getRenderCount();
+  const tokenBefore = getLastColorBgBase();
+
+  act(() => {
+    themeObject.toggleDarkMode(true);
+  });
+
+  expect(getRenderCount()).toBeGreaterThan(rendersBefore);
+  expect(getLastColorBgBase()).not.toBe(tokenBefore);
+});
+
+test('toggleDarkMode updates every concurrently mounted provider for the same theme instance', () => {
+  const themeObject = Theme.fromConfig();
+  const first = makeProbe();
+  const second = makeProbe();
+
+  render(
+    <>
+      <themeObject.SupersetThemeProvider>
+        <first.Probe />
+      </themeObject.SupersetThemeProvider>
+      <themeObject.SupersetThemeProvider>
+        <second.Probe />
+      </themeObject.SupersetThemeProvider>
+    </>,
+  );
+
+  const firstTokenBefore = first.getLastColorBgBase();
+  const secondTokenBefore = second.getLastColorBgBase();
+
+  act(() => {
+    themeObject.toggleDarkMode(true);
+  });
+
+  // Both providers share the same Theme instance, so both must pick up the
+  // toggle -- not just whichever one rendered last.
+  expect(first.getLastColorBgBase()).not.toBe(firstTokenBefore);
+  expect(second.getLastColorBgBase()).not.toBe(secondTokenBefore);
+});
+
+test('a toggleDarkMode call on a different theme instance does not affect a mounted provider', () => {
+  const mounted = Theme.fromConfig();
+  const other = Theme.fromConfig();
+  const { Probe, getRenderCount, getLastColorBgBase } = makeProbe();
+
+  render(
+    <mounted.SupersetThemeProvider>
+      <Probe />
+    </mounted.SupersetThemeProvider>,
+  );
+
+  const rendersBefore = getRenderCount();
+  const tokenBefore = getLastColorBgBase();
+
+  act(() => {
+    other.toggleDarkMode(true);
+  });
+
+  // Each Theme instance owns its own set of provider listeners; toggling a
+  // *different* instance must not re-render a provider mounted from another.
+  expect(getRenderCount()).toBe(rendersBefore);
+  expect(getLastColorBgBase()).toBe(tokenBefore);
+});
+
+test('a toggleDarkMode call after a provider unmounts does not throw and no longer updates it', () => {
+  const themeObject = Theme.fromConfig();
+  const { Probe, getRenderCount, getLastColorBgBase } = makeProbe();
+
+  const { unmount } = render(
+    <themeObject.SupersetThemeProvider>
+      <Probe />
+    </themeObject.SupersetThemeProvider>,
+  );
+
+  const rendersBefore = getRenderCount();
+  const tokenBefore = getLastColorBgBase();
+
+  unmount();
+
+  expect(() => {
+    act(() => {
+      themeObject.toggleDarkMode(true);
+    });
+  }).not.toThrow();
+
+  // The unmounted provider's listener was deregistered, so it shouldn't
+  // have re-rendered (or updated) in response to the toggle.
+  expect(getRenderCount()).toBe(rendersBefore);
+  expect(getLastColorBgBase()).toBe(tokenBefore);
+});
+
+test('a toggleDarkMode call from an ancestor layout effect during the initial commit is not dropped', () => {
+  // Regression harness for the initial-mount race: StorybookWrapper.jsx
+  // toggles the singleton from its own layout effect (ThemeSync) as soon
+  // as a demo mounts. SupersetThemeProvider must have its listener
+  // registered *before* that ancestor effect fires, which only holds if
+  // registration itself runs in a layout effect -- layout effects fire
+  // bottom-up, so this component (nested inside the toggling ancestor)
+  // registers first. If that registration ever regresses to a plain
+  // useEffect, it runs after the ancestor's toggle (passive effects are
+  // deferred until after all layout effects), the notification is
+  // dropped, and this probe would still show the pre-toggle palette.
+  const lightBaseline = Theme.fromConfig();
+  const baseline = makeProbe();
+
+  render(
+    <lightBaseline.SupersetThemeProvider>
+      <baseline.Probe />
+    </lightBaseline.SupersetThemeProvider>,
+  );
+
+  const lightColorBgBase = baseline.getLastColorBgBase();
+
+  const themeObject = Theme.fromConfig();
+  const { Probe, getLastColorBgBase } = makeProbe();
+
+  function AncestorToggler({ children }: { children: ReactNode }) {
+    useLayoutEffect(() => {
+      themeObject.toggleDarkMode(true);
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
+    return children;
+  }
+
+  render(
+    <AncestorToggler>
+      <themeObject.SupersetThemeProvider>
+        <Probe />
+      </themeObject.SupersetThemeProvider>
+    </AncestorToggler>,
+  );
+
+  expect(getLastColorBgBase()).not.toBe(lightColorBgBase);
+});

superset-frontend/packages/superset-core/src/theme/Theme.tsx

@@ -25,7 +25,7 @@ import {
   CacheProvider as EmotionCacheProvider,
 } from '@emotion/react';
 import createCache from '@emotion/cache';
-import { noop, mergeWith } from 'lodash-es';
+import { mergeWith } from 'lodash-es';
 import { GlobalStyles } from './GlobalStyles';
 import {
   AntdThemeConfig,
@@ -145,8 +145,8 @@ export class Theme {
       }),
     } as SupersetTheme;
 
-    // Update the providers with the fully formed theme
-    this.updateProviders(
+    // Update every mounted provider with the fully formed theme
+    this.notifyProviders(
       this.theme,
       this.antdConfig,
       createCache({ key: 'superset' }),
@@ -193,13 +193,29 @@ export class Theme {
     return JSON.stringify(serializeThemeConfig(this.antdConfig), null, 2);
   }
 
-  private updateProviders(
+  // Every currently-mounted SupersetThemeProvider for this Theme instance
+  // registers a listener here (see the useEffect below). A single
+  // "last write wins" callback isn't enough once more than one provider can
+  // be mounted from the same Theme instance at a time -- e.g. multiple live
+  // component demos on one docs page -- since each render would overwrite
+  // the previous provider's callback and only the most-recently-rendered
+  // provider would ever hear about a setConfig/toggleDarkMode call.
+  private providerListeners = new Set<
+    (
+      theme: SupersetTheme,
+      antdConfig: AntdThemeConfig,
+      emotionCache: any,
+    ) => void
+  >();
+
+  private notifyProviders(
     theme: SupersetTheme,
     antdConfig: AntdThemeConfig,
     emotionCache: any,
   ): void {
-    noop(theme, antdConfig, emotionCache);
-    // Overridden at runtime by SupersetThemeProvider using setThemeState
+    this.providerListeners.forEach(listener =>
+      listener(theme, antdConfig, emotionCache),
+    );
   }
 
   SupersetThemeProvider({ children }: { children: React.ReactNode }) {
@@ -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(() => {
+      const listener = (
+        nextTheme: SupersetTheme,
+        nextAntdConfig: AntdThemeConfig,
+        nextEmotionCache: any,
+      ) => {
+        setThemeState({
+          theme: nextTheme,
+          antdConfig: nextAntdConfig,
+          emotionCache: nextEmotionCache,
+        });
+      };
+      this.providerListeners.add(listener);
+      return () => {
+        this.providerListeners.delete(listener);
+      };
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
 
     return (
       <EmotionCacheProvider value={themeState.emotionCache}>