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 [Flight]: Hydration fails when suspended components introduce new client components #24578

Closed
jplhomer opened this issue May 18, 2022 · 4 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@jplhomer
Copy link
Contributor

When hydrating a React Server Components (Hydrogen) app, components which suspend on the server and then introduce a new client components in their resolved body cause hydration to fail in the client.

S1:"react.suspense"
M2:{"id":"ShopifyProvider-LTE5NjIzODc4MTk","name":"ShopifyProviderClient"}
M3:{"id":"BrowserRouter-MTg0MzIyNTE2OQ","name":"BrowserRouter"}
M4:{"id":"RouteParamsProvider-LTkzNzI0NzkzNw","name":"RouteParamsProvider"}
// --- Button loads fine:
M6:{"id":"Button-LTY4MjM2MzAzNg","name":"Button"}
J0:[["$","$1",null,{"fallback":null,"children":["$","@2",null,{"shopifyConfig":{"locale":"EN-US","languageCode":"EN","storeDomain":"hydrogen-preview.myshopify.com","storefrontToken":"3b580e70970c4528da70c98e097c2fa0","storefrontApiVersion":"2022-07"},"children":["$","@3",null,{"children":["$","@4",null,{"routeParams":{},"children":["$","div",null,{"className":"wrapper","children":[["$","div",null,{"className":"hello","children":"Hello World"}],["$","$1",null,{"fallback":"Loading...","children":["@5",["$","@6",null,{"children":"This does not cause a hydration error"}],["$","p",null,{"children":"Plain text"}]]}]]}]}]}]}]}],["$","$1",null,{"fallback":null,"children":"@7"}]]
// --- Button2 dynamic imports, suspends, and causes hydration mismatch:
M8:{"id":"Button2-MTEwMzA2MzgzOA","name":"Button2"}
M9:{"id":"AnalyticsErrorBoundary-LTE0MzI5ODE2Nzg","name":"default"}
Ma:{"id":"Analytics-MTI5NjgyNTAxMQ","name":"Analytics"}
J5:["$","div",null,{"className":"suspended-server","children":[["$","p",null,{"children":["Shop: ","Snowdevil"]}],["$","@8",null,{"children":"This DOES cause a Hydration error"}]]}]
J7:["$","@9",null,{"children":["$","@a",null,{"analyticsDataFromServer":{"url":"http://localhost:3000/__rsc?state=%7B%22pathname%22%3A%22%2F%22%2C%22search%22%3A%22%22%7D","normalizedRscUrl":"http://localhost:3000/"}}]}]

This is presumably because the client reader throws a Promise while dynamically importing the new client component, and the hydration process gets confused about the mismatch between SSR (since resolved) and client.

I think this issue is potentially related: #24384 and I was hoping @gnoff's fix would also resolve this issue, but it still persists.

We can "fix" the issue by wrapping all client components with <Suspense>, but that does not seem like the correct solution, and we don't want to provide this as the answer to developers building on Hydrogen if it's not the expected behavior.

I'm using React version: ~18.2, using the build from this PR: #24480

Steps To Reproduce

It's been a real struggle to reproduce this, especially since there are not examples out there for SSR hydration while using React Server Components.

However, I've created a stripped-down repro in the Hydrogen repository which hopefully helps trace this error:

  1. Clone shopify/hydrogen and check out this PR: [ci] release 2023-07 Shopify/hydrogen#1295
  2. Run yarn && yarn dev.
  3. Run cd templates/template-hydrogen-hello-world && yarn dev

Link to code example: Shopify/hydrogen#1295

The current behavior

It depends on the order of elements. Sometimes:

Error: Hydration failed because the initial UI does not match what was rendered on the server.
    at throwOnHydrationMismatch (react-dom.development.js:12533:9)
    at tryToClaimNextHydratableInstance (react-dom.development.js:12561:7)
    at updateHostComponent (react-dom.development.js:20327:5)
    at beginWork (react-dom.development.js:22120:14)
    at beginWork$1 (react-dom.development.js:28241:14)
    at performUnitOfWork (react-dom.development.js:27340:12)
    at workLoopConcurrent (react-dom.development.js:27326:5)
    at renderRootConcurrent (react-dom.development.js:27288:7)
    at performConcurrentWorkOnRoot (react-dom.development.js:26521:38)
    at workLoop (scheduler.development.js:266:34)

Other times (if the elements are the same tag):

Error: Text content does not match server-rendered HTML.
    at checkForUnmatchedText (react-dom.development.js:9696:11)
    at diffHydratedProperties (react-dom.development.js:10359:13)
    at hydrateInstance (react-dom.development.js:11332:10)
    at prepareToHydrateHostInstance (react-dom.development.js:12590:23)
    at completeWork (react-dom.development.js:22628:17)
    at completeUnitOfWork (react-dom.development.js:27379:16)
    at performUnitOfWork (react-dom.development.js:27351:5)
    at workLoopConcurrent (react-dom.development.js:27326:5)
    at renderRootConcurrent (react-dom.development.js:27288:7)
    at performConcurrentWorkOnRoot (react-dom.development.js:26521:38)

The expected behavior

No hydration errors.

@jplhomer jplhomer added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label May 18, 2022
@jplhomer
Copy link
Contributor Author

Hi @gnoff or @sebmarkbage — curious if either of you could give this a couple minutes of attention when you have a moment, as you can likely determine pretty quickly whether this is a problem in Hydrogen or in React. We'd really appreciate it ❤️

@gnoff
Copy link
Collaborator

gnoff commented May 19, 2022

@jplhomer will check this out today

@gnoff
Copy link
Collaborator

gnoff commented May 20, 2022

Just a quick update, I know what React is doing that is causing these errors to leak but I don't yet know why it is doing this. Essentially the Root is completing normally when I would have expected it to Suspend which causes the hydration errors to become recoverable which is why you see them in the console. I'm going to keep looking at it but hunch is, not an issue in hydrogen

@jplhomer
Copy link
Contributor Author

Just confirmed that #24578 fixes the issue 🎉 can reproduce the fix by using react@https://react-builds.vercel.app/api/prs/24532/packages/react and react-dom@https://react-builds.vercel.app/api/prs/24532/packages/react. THANK YOU!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

2 participants