fix: prevent unwanted rerender

[Bram-Zijp]

I noticed that after initial render of the ThemeColorProvider, it was re-rendering.This MR prevents this re-render from taking place by populating initial color state with the localStorage cache state on first render.

Edit: It now just uses the classNames to populate initial state.

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/systemui/theme-ui/HnJbs5nGMiJR6EZNZh9V4wx6ECbh
✅ Preview: https://theme-ui-git-fork-bram-zijp-fix-remove-an-unwanted-4bbf16.vercel.app

[lachlanjc]

Super appreciate the PR! I don't 100% understand what's happening in the new code though: why do we need to remove the body class on first render?

[Bram-Zijp]

At the bottom of the changed file, a class is set on the body. This specifies the initial color mode based on the local storage data, I believe. To be honest, I'm not sure if we need this "body className" in the first place. Perhaps this is something to look into. My gut feeling is that this body className is used, to consume server side rendered styles, bound to this body className.

In terms of change, the code does the exact same thing as what was implemented, with the exception of the re-render.

Why not remove classes on the useState? I believe this action will mutate the DOM while still in the process of rendering, whereas mutating the dom in the useEffect will happen after rendering of the initial DOM is complete. Correct me if I'm wrong.

[Bram-Zijp]

Instead of using getModeFromClass, we could specify a global on the same place we set the theme className, obtained from localStorage.. I think this will result in less LoC and better performance (noticable or not).

[lachlanjc]

Thanks for the explanation! @hasparus could you take a look?

[hasparus]

Thanks for the PR @Bram-Zijp! Nice catch!

@lachlanjc I'll need to take a look at corner cases (how does it look like with SSR? that rerender triggers update) and merge a few outstanding PRs to this part first (fcis' fixes).