-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactor ThemeProvider and prefers-color-scheme detection #2
Comments
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 I flipped back and forth a couple of times, and then this morning settled on this...
Thanks to 1. above, And since Except they can't (see below). I then updated 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 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 [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 Thinking time (and a break) |
Okay so just in case anyone stumbles across this - the issue of where to place 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 Hence @chaance 's attempt to fix this with an additionally injected script and the 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 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] |
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 Take a look at entry.server.tsx , root.tsx and ThemeProvider for the key changes. What a journey. |
[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.
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.
The text was updated successfully, but these errors were encountered: