react icon indicating copy to clipboard operation
react copied to clipboard

setColorMode not working within useEffect

Open chadfawcett opened this issue 3 years ago • 4 comments

Describe the bug I'm attempting to store the user's preferred color mode in local storage, and set the color mode upon loading the page. My approach was to use a useEffect in my ThemeSwitcher component that loads the mode from local storage and calls setColorMode. It appears setColorMode is not working as I'd expect when called from within a useEffect. I would expect the component to re-render with colorMode set to night. It does work within a button click handler though.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://stackblitz.com/edit/react-yz4cdm?file=src/ThemeSwitcher.js
  2. The colorMode defaults to 'day, and I try to set it to night when the component mounts (useEffect`). This does not happen.
  3. I create my own MyContext that stores a number val defaulting to 0. When the component mounts, it increases the val to one, which is how I'd expect useState to handle the value change.

Expected behavior I expected the component to render in day mode at first, but then the setColorMode in useEffect would update the colorMode value which would cause a re-render in night mode

Screenshots https://stackblitz.com/edit/react-yz4cdm?file=src/ThemeSwitcher.js

Desktop (please complete the following information):

  • OS: MacOS
  • Browser: Chrome
  • Version: Version 98.0.4758.80

chadfawcett avatar Feb 14 '22 22:02 chadfawcett

Thanks for the bug report, @chadfawcett! We'll look into this. Here are a couple option that might unblock you:

  • Since localStorage.getItem() is synchronous you could fetch the stored color mode when rendering ThemeProvider and pass it directly as a prop:

    const storedColorMode = localStorage.getItem('color_mode');
    return (
     <div className="App">
       <ThemeProvider colorMode={storedColorMode || 'light'}>
    

    This implementation also prevents a "flash of light mode" on page load.

    https://stackblitz.com/edit/react-s6hzbd?file=src%2FApp.js

  • If you can't pass colorMode to ThemeProvider directly, wrapping setColorMode with a setTimeout inside useEffect seems to work:

    useEffect(() => {
      console.log('Setting color mode');
      setTimeout(() => setColorMode('night'));
      setVal(val + 1);
    }, []);
    

    https://stackblitz.com/edit/react-gvc6so?file=src%2FThemeSwitcher.js

colebemis avatar Feb 15 '22 00:02 colebemis

Thanks for the suggested workarounds @colebemis. This is where my whole setup started to have an affect beyond the sandbox. I'm doing this within a Next.js application, so the initial rendering happens server side, where local storage doesn't exist, so it defaults to auto ("system" or light in my case). I tried a timeout, even with an actual timeout of one or two seconds didn't work in the Next/js app.

The color mode is applied properly when the "fast refresh" happens in dev mode, which skips the server side rendering. So perhaps my problem is more specific to Next.js in that I need to force it to re-render client side to pick up the changed color mode.

chadfawcett avatar Feb 15 '22 01:02 chadfawcett

I forgot to mention I'm also using @primer/css, which may also be the culprit here. Since I'm setting a data attribute on an HTML element to specify data-color-mode, Next.js complains that this is different from the SSR result.

If I remove the data attribute code, I then get a hydration error around the BaseStyles wrapper component. My assumption is that the BaseStyles component doesn't like having the colorMode on the ThemeProvider being different from the SSR.

I'll dig some more into my specific case, but wanted to provide a bit more context before sending you down too much of a rabbit hole. The original issue of setColorMode within a useEffect seems like a bug, but with it fixed, it may or may not solve my specific use case.

chadfawcett avatar Feb 15 '22 01:02 chadfawcett

Another update: I got the useEffect + setTimeout working! So most of the above can be ignored 😅.

Just the originally posted issue.

chadfawcett avatar Feb 15 '22 17:02 chadfawcett

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Aug 14 '22 18:08 github-actions[bot]

Not sure if this was resolved or not. Can someone look into it and re-open if it's still valid?

chadfawcett avatar Aug 22 '22 21:08 chadfawcett

@colebemis since you're on FR this week and familiar with this issue, could you take a peek and re-open if this issue is still valid? Thank you!

lesliecdubs avatar Aug 23 '22 03:08 lesliecdubs