infonomic / remix.infonomic.io

A Remix demo app with CSS, Tailwind, Radix UI and other headless UI components.
https://remix.infonomic.io
MIT License
46 stars 5 forks source link

Refactor ThemeProvider and prefers-color-scheme detection #2

Open 58bits opened 1 year ago

58bits commented 1 year ago

[Update - for the TL;DR - jump to this comment ]

@mattstobbs @kiliman - so I took a stab at refactoring Matt's excellent theme provider so that it can be placed inside the body of the React component tree.

I'm sure there's hours of ROFL if you look too closely at my commit history and how I ended up with what's here now. Honestly Matt - what a journey you went on. It took a while to unpack things and put them back together again, but I think it just about works. I'm using a data attribute on the html element as a guard, indicating whether the theme has been set yet and to allow a default / fallback theme when JavaScript is disabled. I tried using the theme switcher event handler to persist the theme session directly, but it really does have to be done the way you've done it - or there will be mismatched server and client classes in the theme switcher itself.

Theme-Provider

Most of the code lives here... https://github.com/infonomic/remix.infonomic.io/tree/develop/app/ui/theme/theme-provider

An even cooler solution might be to put a Remix-style form around the theme switcher button and submit the action to set the theme session there - which would mean the entire thing would work fine without client-side JS (it almost does now).

Would be interested to know what you guys think. Thoughts or suggestions most welcome - and no hard feelings at all if this is all a terrible idea. heh.

I've reverted back to a 'normal' entry.server.tsx for now Michael - although I've seen your updates here https://github.com/kiliman/remix-hydration-fix and will be testing this with the new setup soon.

Hope some of this helps.

58bits commented 1 year ago

Okay well this was educational.

In my first attempt above I tried to a) allow the provider to be placed inside the body and not wrap the entire document, as well as b) make the theme switcher a little more 'event driven', i.e. not require useEffect for mount and persist, as well as change the signature for setTheme so that it wasn't directly setting state in the provider via Dispatch and SetStateAction.

I flipped back and forth a couple of times, and then this morning settled on this...

  1. Detector: Create an early 'Detector' function - removing the responsibility of early theme detection from ThemeProvider.

  2. Injector: Create an InjectPrefersTheme component to be placed in the head, but which doesn't rely on useTheme - a slightly different version of Matts' NonFlashOfWrongThemeEls (and which @chaance has refactored to ThemeHead)

  3. Provider: Place ThemeProvider in the body

Thanks to 1. above, ThemeProvider theme prop can now never be null - so I removed the useState initializer function - as the provider will always receive a theme.

And since ThemeProvider theme state and the context value for theme can never be null - the theme switch, and any other component that uses useTheme can rely on the theme value from the ThemeProvider context.

Except they can't (see below).

I then updated setTheme in the the context for ThemeProvider as per the stated goal in the opening para above.

You can see all of the above in action here...

https://github.com/infonomic/remix.infonomic.io/blob/develop/app/root.tsx

And then I crashed straight into this.

Because we manipulated the DOM in our faux server 'inject script' (via InjectPrefersTheme)- we fixed the html class for theme so there is no longer a mismatch from the server default and our detected prefers-color-scheme (and therefore no hydration error) - for the html class at least. But, on a first site visit when no cookie has been set, or if cookies were not allowed - we'd effectively have to do the same everywhere else that the theme value from the useTheme hook is being used to conditionally render any classes, attributes, or elements since the initial render for all of this will always be the SSR value - and so any component that wants to use useTheme and the theme value to conditionally render classes, attributes, elements etc. - will generate a 'did not match' Server / Client hydration error.

And then just a few minutes later, after re-reading Matts' original post, I discovered the work that Chance Strickland @chaance has done here, which attempts to solve the problem I crashed into...

https://github.com/remix-run/examples/blob/main/dark-mode/app/utils/theme-provider.tsx

...with the addition of his Themed component containing additional logic and a new clientThemeCode script.

[scratches head]

... and so the big question now is whether there is any value in attempting to place the ThemeProvider in the body as I've done here - the impetus for which was @kiliman and his effort to solve hydration errors, as well as really just to see if it could be done, and to learn a little more about Remix.

It all pretty much works - with the exception of the useTheme hook, which @chaance has provided a solution for via Themed.

Thinking time (and a break)

58bits commented 1 year ago

Okay so just in case anyone stumbles across this - the issue of where to place ThemeProvider (or any provider, and whether it's a good idea for them to wrap the entire document or not), and the issue of attempting to set an initial theme based on 'prefers-color-scheme' - are actually two separate issues.

The main issue with 'prefers-color-scheme' is that the server (at the moment - see below) has no way of knowing this value, and so on a first visit to a site, and during first SSR, we set a default (or none) and then 'fix it' using an injected script before hydration. The result is that on a first SSR it's not just the html class element that needs fixing - it's any conditionally rendered class, attribute or element - because useTheme and the provider context values during SSR, won't be the same during client hydration and client render.

Hence @chaance 's attempt to fix this with an additionally injected script and the Themed component.

The problem here would be the same for any framework trying to reconcile something that can't be known during SSR.

It turns out that server-side detection of 'prefers-color-scheme', along with client 'prefers media features' - for motion, contrast, transparency etc., is actively being worked on by WICG - User Preference Media Features Client Hints Headers. It's also supported by Chrome (and Opera) - https://caniuse.com/mdn-http_headers_sec-ch-prefers-color-scheme

So - there are fixes for the hydration issue, and early detection of 'prefers-color-scheme' - like @mattstobbs ' solution for injecting a script to 'fix' the html class before client hydration occurs (and prevent a flash of wrong theme).

The fixes for useTheme and any theme conditionally rendered client code on first render are a little more nuanced, and certainly @chaance solution is a valid one.

Still thinking about all of this, and will experiment a little more before deciding what to do next for our demo application.

[P.S. Apologies in advance @chaance if you simply committed the example app and weren't the author of the current solution]

58bits commented 1 year ago

So in the end we opted for a simpler solution - only detecting 'prefers-color-scheme' via User Preference Media Features Client Hints Headers which is currently supported by both Chrome and Edge - https://caniuse.com/mdn-http_headers_sec-ch-prefers-color-scheme.

This removes a lot of complexity and workarounds, and removes all SSR / client hydration issues. We think it's worth it - but of course there are still tradeoffs. If we can't detect 'prefers-color-scheme' via Sec-CH-Prefers-Color-Scheme - we fallback to a default them, and then let ThemeProvider (and switcher) take things from there with the user able to manually set a preferred theme (including more than just dark and light - dark, light, flashy etc.).

Take a look at entry.server.tsx , root.tsx and ThemeProvider for the key changes.

What a journey.