Skip to content
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

[BUG] Fix SSR hydration errors #920

Closed
jplhomer opened this issue Mar 15, 2022 · 3 comments · Fixed by #1571
Closed

[BUG] Fix SSR hydration errors #920

jplhomer opened this issue Mar 15, 2022 · 3 comments · Fixed by #1571
Labels
bug Something isn't working
Milestone

Comments

@jplhomer
Copy link
Contributor

jplhomer commented Mar 15, 2022

Describe the bug
In the current release branch, pretty much every page has this issue:

Screen Shot 2022-03-15 at 2 15 46 PM

I've been able to eliminate this hydration error by wrapping any...

  1. server components
  2. ...that suspend
  3. ...which are passed as children to client components

... with a <Suspense> boundary.

This might be a bug in Hydrogen. This might be a bug in React.

However, I have confirmed with the React team that this behavior is not intended (e.g. you should be able to pass server components to client components as children which suspend without needing a boundary inside the bounds of the client component).

It is very difficult to tell, because we are many experimental releases behind, and a lot of work has gone into how Suspense & Hydration works.

I've attempted to fix this in #907 but I've failed, because something is messing with the E2E tests locally. This fix is not correct, either, because it suspends at the router level with a fallback of null. This causes the page to flash from rendered (SSR) to fallback (RSC) to rendered (RSC). This is not good.

One might impose a rule on Hydrogen developers that they should NOT use useShopQuery or anything that suspends at the top level page server component, but rather provide a Suspense boundary with a fallback which matches their page design and render another component in the same file that does suspend. However, this feels like a weird rule to impose if it's not an actual restriction and just a bug with our implementation.

To Reproduce

  • Visit a page in the main branch
@jplhomer jplhomer added the bug Something isn't working label Mar 15, 2022
@jplhomer jplhomer added this to the RC1 milestone Mar 15, 2022
@karissagenovese
Copy link

@jplhomer Hey! Do you have a sense of when you expect this bug to be fixed? It isn't a blocker for Website Platform this week but we will need this done for a release we have next Thursday.

@jplhomer
Copy link
Contributor Author

@karissagenovese I do not unfortunately. Can you tell me more about what this would be blocking? This hydration issue affects initial SSR page loads and reverts to client-side rendering (which is fine). Short of a console warning, I'm curious what else this affects.

@jplhomer
Copy link
Contributor Author

jplhomer commented May 25, 2022

Update: there's a fix for this issue landing soon in React! 🎉 facebook/react#24532

I'd anticipate it to be released in React 18.2.

To get the fix now, you can use an experimental version of React:

yarn add react@https://react-builds.vercel.app/api/prs/24532/packages/react react-dom@https://react-builds.vercel.app/api/prs/24532/packages/react-dom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants