React 18 Support: Cleanup DOM during hydration to avoid mismatch

[mgreenw]

Fixes #260 (hopefully 😅)

Hey folks! I've been following this issue for a while and am hoping to continue using @artsy/fresnel on React 18 and beyond. Even with current React 18 support via disableDynamicMediaQueries, @artsy/fresnel is still the best way I've found to dynamically SSR different markup for different devices.

I recently stumbled on this gist which demonstrates a method for avoiding hydration mismatches by using a combination of useId and some initial-render logic to clean up the DOM during hydration. While the example in the gist isn't realistic because it causes a Flash of Server Rendered Content™, I was intrigued by the idea and set out to see if I could integrate it into @artsy/fresnel.

While most of the previous efforts to resolve #260 have been related to using <Suspense> boundaries in clever ways, this method of DOM cleanup is a supported solution based on @gaearon's comment on the React issue:

The supported solutions are:

• Render all variants and let all effects fire.
• Or patch up the HTML tree to prune the invisible branches before starting hydration.

TL;DR: I was successful at using this method within @artsy/fresnel to cleanup the DOM during hydration! I hope this resolves the issue going forward.

Some notes:

• This solution only supports React 18+ because it requires useId. Folks that need React 17 support can continue using an older version of @artsy/fresnel
• I refactored Media to a functional component instead of a class component, which drastically simplified it by allowing use of useContext instead of multiple nested MediaParentContext.Provider elements. However, by refactoring it in this way, it is no longer possible to determine the parent component's name for the "at is being used with the largest breakpoint." warning. I think this is a reasonable price to pay for simplicity and a working implementation for React 18!
• I have tested this approach on the kitchen-sink example and it works! I had to make a few tweaks to the example to properly call renderChildren on the inner content of rendered nodes with className. I believe that example was not using renderChildren properly. It may be important to expand on this usage in the README, but it should be possible with this approach to pass a custom render function that responds with multiple children nested in a fragment, as long as they all receive className and use renderChildren dynamically on their children.
• I have published a version of this to npm at @mgreenw/fresnel:7.0.0-alpha.2 for folks to try out / use for testing! Artsy team (@damassi): if you are able to publish your own alpha version based on this PR, that would be super helpful so others only need to change the package version and not the name for testing.
• The unit tests are all passing, which is great, but I saw that there were some TODOs around unit testing around hydration. Please let me know if there's anything I can do there to add testing -- it seems like it's tricky or not possible with the current setup?

Please let me know if you have feedback, and I would really appreciate folks testing this out on their own projects to see if you run into any issues at scale.

[damassi]

Oh my @mgreenw! PRs like this are as good as it gets. Checking it out now 👀

[damassi]

👌 Much less code

[damassi]

Simple, easy to understand javascript saves the day again. It is almost ironic.

[damassi]

Thank you so much

[artsyit]

🚀 PR was released in v7.0.0 🚀

[roderickhsiao]

Solid nice work thank you!!!

[mgreenw]

Thanks for getting this reviewed and released so fast @damassi! 😊 I hope this fix works for everyone -- very curious to hear if anyone has issues.

[gaearon]

Tbh I don't really understand the solution attempted here but I do want to warn that the notion of "first render" is not very well-defined. React may throw away a partially rendered tree if it suspends, and then start again from scratch. If the tree is thrown away, so are the refs. So your useRef(true) will again think it's "first render". If you're resilient to that maybe it's ok. Also, Strict Mode always renders twice, and one of the renders gets thrown away as well.

[gaearon]

I guess if the code itself is safe to run many times then it doesn't matter if it tries to run again when the branches have already been pruned.

[damassi]

Thanks for the heads up @gaearon 🙏

[mgreenw]

Agreed, thanks for pointing that out @gaearon. This code should be resilient to multiple runs because the conditional logic for "should we prune this branch?" is based off of the device width, which hopefully won't change while React is still passing or re-passing through the tree.

I wonder if there's a case where some other conditional suspense-related logic could cause the tree to change in a way that causes a mismatch. The worst that could happen is a full client re-render, but it might be good to try and force that case and see if there's a way to avoid it.

[gaearon]

which hopefully won't change while React is still passing or re-passing through the tree.

It can change. Imagine we're starting a render and then run into a lazy() component for which the logic has not loaded yet. React will throw away that render attempt. However, React will not replace the server-rendered HTML with a fallback yet — that's the part enabled by React 18. Instead, React will attempt another render when the code loads. But the user can change the window width in between these two events.

[gaearon]

I think it would be good to simulate this (e.g. with a lazy that's given a Promise that resolves artifically slowly) and see what happens.

[gaearon]

In either case it seems like the worst case is falling back to a client render so it's not too bad regardless. If the happy case works.

[Geoff-Ford]

This is amazing, thank you so much @mgreenw! We have been released 🙌

[ehowey]

Congrats this is huge! Was waiting for React 18 support!