Skip to content
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.

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

Values derived from useColorMode() can be stale when rendering in prod mode #7986

Open
6 of 7 tasks
fvsch opened this issue Aug 22, 2022 · 13 comments
Open
6 of 7 tasks
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. domain: theme Related to the default theme components
Milestone

Comments

@fvsch
Copy link

fvsch commented Aug 22, 2022

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

The value returned by the useColorMode hook from @docusaurus/theme-common seems to have a strange behavior when:

  1. Building for production and/or using React in prod mode (npm run build && npm run serve).
  2. window.localStorage.theme is 'dark' while the themeConfig.colorMode.defaultMode value is 'light', or window.localStorage.theme is 'light' while the themeConfig.colorMode.defaultMode value is 'dark'.

Then trying to use the colorMode value set the value of DOM attributes ends up generating a DOM with incorrect attribute values:

import { useColorMode } from "@docusaurus/theme-common";
import React from "react";

export default function ColorModeTest() {
  const { colorMode } = useColorMode();
  return (
    <div title={colorMode}>{colorMode}</div>
  );
}

In the conditions described above, if the themeConfig.colorMode.defaultMode value is 'light' and the localStorage.theme value is 'dark':

  1. The generated HTML will be: <div title="light">light</div>
  2. The hydrated DOM will be: <div title="light">dark</div>

This is particularly troublesome when you have UI components that support both dark and light themes and rely on HTML attributes to set the correct color theme on the component itself (e.g. in order to display a light component in a dark context for visual emphasis).

Reproducible demo

https://github.com/fvsch/docusaurus-use-color-mode-stale-value

Steps to reproduce

  1. Check out https://github.com/fvsch/docusaurus-use-color-mode-stale-value
  2. Install dependencies, build for prod and serve that build: npm install && npm run build && npm run serve
  3. The home page of the demo should be using a light theme.
  4. Click the theme switching button on the top right to switch to the dark theme.
  5. Reload the page.

Expected behavior

The values derived from the colorMode, whether they're text nodes or attribute nodes, should be in sync and reflect the localStorage.theme value.

In the last step, the visual result should be:

dev-mode-3-localstorage-theme-dark

Actual behavior

Attribute values that are derived from the colorMode value seem to be outdated.

In the last step, the visual result is:

prod-mode-3-localstorage-theme-dark

Your environment

Self-service

  • I'd be willing to fix this bug myself.
@fvsch fvsch added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Aug 22, 2022
@slorber
Copy link
Collaborator

slorber commented Aug 24, 2022

Quick StackBlitz repro link: https://stackblitz.com/github/fvsch/docusaurus-use-color-mode-stale-value?file=src%2Fcomponents%2FColorModeTest%2Findex.js


Agree there is likely a React hydration problem.

const getInitialColorMode = (defaultMode: ColorMode | undefined): ColorMode =>
  ExecutionEnvironment.canUseDOM
    ? coerceToColorMode(document.documentElement.getAttribute('data-theme'))
    : coerceToColorMode(defaultMode);

  const [colorMode, setColorModeState] = useState(
    getInitialColorMode(defaultMode),
  );

Again related blog post: https://www.joshwcomeau.com/react/the-perils-of-rehydration/

Afaik it has been claimed in a clearer way more recently by the React core team that the SSR/CSR must 100% match. React 18 introduces a onRecoverableError callback which usually notice you of hydration mismatches, and in this React 18 POC PR (#7855) I noticed we had some.

In this case I think we can't technically init our React state with the correct value, but instead should always init it to the default color mode, and then trigger a new re-render to fix it after hydration.

const [colorMode, setColorModeState] = useState(defaultMode);

useLayoutEffect(() => {
  coerceToColorMode(document.documentElement.getAttribute('data-theme'))
},[])

This should fix the SSR/CSR mismatch, but this is a bit annoying unfortunately, as it will mean some components will render first with the wrong colorMode and then eventually re-render with the right one

@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Aug 24, 2022
@Josh-Cena Josh-Cena added domain: theme Related to the default theme components difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. labels Aug 24, 2022
@dionysuzx
Copy link

any status update or timeline on this? seems to be an important one. I have an logo with a font color that I need to change dynamically on the current color theme, and currently this bug means I need to keep the theme switcher toggle disabled for now. Thanks!

@slorber
Copy link
Collaborator

slorber commented Oct 26, 2022

I'll probably fix this as part of the React 18 upgrade, as we'll be able to see hydration error messages that I would have to fix one by one to complete this migration.

Note you might not need useColorMode, you can also use html[data-theme="dark" CSS selectors and we also have a ThemedImage component: https://docusaurus.io/docs/markdown-features/assets#themed-images

@slorber slorber added this to the 3.0 milestone Sep 25, 2023
@slorber slorber added apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee and removed apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee labels Sep 25, 2023
@slorber slorber modified the milestones: 3.0, 3.x Oct 8, 2023
nk-coding added a commit to MiSArch/docs that referenced this issue Dec 1, 2023
- upgrades dependencies
- fixes lightbox css
- lightbox uses two class selectors with the same specificity, thus, the
priority is based on the order
- however, webpack merges some selectors and changes the priority in a
production build
  - as a fix, added the important selector with more specificity
- also adds a workaround for
facebook/docusaurus#7986
@sszczep
Copy link

sszczep commented Feb 1, 2024

Is there any ETA on that? Still broken on 3.0.1

@slorber
Copy link
Collaborator

slorber commented Feb 1, 2024

@sszczep this is likely not something we can actually fix. Due to how React hydration works, the first value returned by this hook is likely not the actual effective color mode your page is rendered in.

I suggested in #7986 (comment) to use CSS media queries for styling instead.

If you have another use-case that can't be solved with CSS, please explain why and I'll try to provide a workaround.

@sszczep
Copy link

sszczep commented Feb 1, 2024

@sszczep this is likely not something we can actually fix. Due to how React hydration works, the first value returned by this hook is likely not the actual effective color mode your page is rendered in.

I suggested in #7986 (comment) to use CSS media queries for styling instead.

If you have another use-case that can't be solved with CSS, please explain why and I'll try to provide a workaround.

I need to pass current color mode to Material UI Theme Provider. Can't use CSS variables unfortunately. If you have a workaround that would be great. Cheers

Edit: Turns out that you can use MUI with CSS theme variables. Please check my other reply in that thread: #7986 (comment)

@slorber
Copy link
Collaborator

slorber commented Feb 1, 2024

There is no good workaround IMHO.

You might be fine with the following, but use at your own risks, and be aware it is likely to create issues down the line.

const isDarkMode = document.documentElement.getAttribute('data-theme') === "dark";

Please also understand that Docusaurus doesn't have good support for "legacy CSS-in-JS libs" (StyledComponents, Emotion, JSS, MUI...), that require collecting styles during the rendering process (see also #3236).

So I'd simply recommend not using MUI in the first place, unless you clearly understand the tradeoff you make and the risk of having flashes of unstyled content.

@sszczep
Copy link

sszczep commented Feb 1, 2024

Turns out that with MUI v5.6.0 they released a support for CSS theme variables. I successfully incorporated CssVarsProvider with Docusaurus theming system. They are now synced without any further gimmicks and shouldn't cause issues in the future. As described here it has some drawbacks (bigger code size, polluting stylesheets with vars) but it is definitely better than dealing with FOUC.

@slorber I also tested your recommendation, worked fine with MutationObserver, but the flicker was worse than I initially anticipated.

If anyone is interested, here is the code:
src/theme/Layout/MuiThemeProvider.tsx

import React from "react";

import {
  Experimental_CssVarsProvider as CssVarsProvider,
  experimental_extendTheme as extendTheme,
} from "@mui/material/styles";

const theme = extendTheme({
  // custom theming overrides...
});

export default function MuiThemeProvider({
  children,
}: React.PropsWithChildren) {
  return (
    <CssVarsProvider
      theme={theme}
      attribute="data-theme"
      colorSchemeStorageKey="theme"
      modeStorageKey="theme"
    >
      {children}
    </CssVarsProvider>
  );
}

src/theme/Layout/index.tsx

import React from "react";

import Layout from "@theme-original/Layout";
import { Props } from "@theme/Layout";

import MuiThemeProvider from "./MuiThemeProvider";

export default function LayoutWrapper({ children, ...props }: Props) {
  return (
    <Layout {...props}>
      <MuiThemeProvider>
        {children}
      </MuiThemeProvider>
    </Layout>
  );
}

Once again, thanks for guiding me into that direction.

@LeonnardoVerol
Copy link

I'll probably fix this as part of the React 18 upgrade, as we'll be able to see hydration error messages that I would have to fix one by one to complete this migration.

Note you might not need useColorMode, you can also use html[data-theme="dark" CSS selectors and we also have a ThemedImage component: https://docusaurus.io/docs/markdown-features/assets#themed-images

I had the same problem and "ThemedImage" helped

@slorber
Copy link
Collaborator

slorber commented Feb 28, 2024

@LeonnardoVerol which problem, and can you create a https://docusaurus.new repro please so that we see this problem in action?

@LeonnardoVerol
Copy link

@LeonnardoVerol which problem, and can you create a https://docusaurus.new repro please so that we see this problem in action?

It was like the original post... I was using "colorMode" to swap 2 images.
Docusaurus would swap correctly the images on light/dark toggle but fail to swap the images in some other situations. (E.g: on page refresh)
Replacing that logic for "ThemedImage" solved my problem.

@glyph-cat
Copy link

any status update or timeline on this? seems to be an important one. I have an logo with a font color that I need to change dynamically on the current color theme, and currently this bug means I need to keep the theme switcher toggle disabled for now. Thanks!

There seems to be a hacky way to temporarily overcome this problem, which is by wrapping the base component in another component that delays the rendering using useEffect, so that the obtained value will be the correct one. But it might not be suitable for all use cases. For mine, it happened to be a theme switcher toggle as well so it worked… at least for now.

const visibilityReducer = () => true

function SomeComponent() {
  const [visible, setVisibilityTrue] = useReducer(visibilityReducer, false)
  useEffect(setVisibilityTrue, [setVisibilityTrue])
  return visible ? <SomeComponentBase /> : null
}

Also, this seems to be quite an old issue and I see some PRs were merged above. My @docusaurus/core and @docusaurus/preset-classic are both v3.4.0, but I am encountering this issue also, not sure if this is normal? Thought of opening a new issue at first, but changed my mind thinking it might be a better idea to continue in this thread for now.

calejvaldez added a commit to ojosproject/website that referenced this issue Jun 11, 2024
A React hydration issue made the light-mode version of the
hover colors appear on dark-mode version of the website, making
it way too bright. Therefore, we're just going to be using a
box-shadow to make it easier on the eyes.

Resolves #36
Ref: facebook/docusaurus#7986
@EaveLuo
Copy link

EaveLuo commented Jun 20, 2024

it's also happen in 3.4, use ThemedImage and tailwind instead of useColorMode, it's work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. domain: theme Related to the default theme components
Projects
None yet
Development

No branches or pull requests

8 participants