react icon indicating copy to clipboard operation
react copied to clipboard

Bug: Hydration mismatch error due to plugins generating script tag on top

Open yongdamsh opened this issue 2 years ago • 77 comments

React version: 18.0.0, 18.1.0-next-fc47cb1b6-20220404 (latest version in codesandbox)

Steps To Reproduce

  1. Install a plugin that creates a script tag at the top(ex: Apollo Client Devtools)
  2. Go to the demo in the new SSR suspense guide
  3. Open preview in a new window
  4. UI mismatch error occurs at hydration time
스크린샷 2022-04-24 오전 11 02 34

Link to code example: https://codesandbox.io/s/kind-sammet-j56ro?file=/src/App.js

The current behavior

If a script tag is inserted before the head tag due to the user's browser environment such as a plugin, it is judged as a hydration mismatch and the screen is broken.

https://user-images.githubusercontent.com/4126644/164953071-14546c74-d9ab-4a6f-8f99-6712f29c6dd6.mov

The expected behavior

This problem may be a part that each third party needs to solve, but I'm wondering if it's possible to handle an exception in the hydration matching logic of React.

yongdamsh avatar Apr 24 '22 02:04 yongdamsh

Might be related to (or same as) https://github.com/facebook/react/issues/22833, but let's keep both open for now

gaearon avatar Apr 24 '22 02:04 gaearon

Is the hydrateRoot function expecting the whole html document to be exactly what gets rendered by renderToPipeableStream? The best would be to just try to hydrate the React root element where the app is rendered?

It does seem that to use renderToPipeableStream I need to render the whole HTML document with a React component, but this is not ideal when e.g. using Vite with SSR in development, since it needs to transform the html to inject custom scripts.

marcusthelin avatar Apr 24 '22 21:04 marcusthelin

From my understanding, if anything else other than document was passed in into hydrateRoot, it doesn't seem to crash when I have chrome extensions that modify the DOM installed (e.g. Dark Reader / Apollo DevTools).

Here is the code sandbox: https://codesandbox.io/s/react-18-root-div-hydrateroot-1f5d5q?file=/src/Html.js:193-941

In the above example, I changed the following:

Html.js

export default function Html({ assets, children, title }) {
  return (
    <html lang="en">
      <head>
        <meta charSet="utf-8" />
        <meta name="viewport" content="width=device-width, initial-scale=1" />
        <link rel="shortcut icon" href="favicon.ico" />
        <link rel="stylesheet" href={assets["main.css"]} />
        <title>{title}</title>
      </head>
      <body>
        <noscript
          dangerouslySetInnerHTML={{
            __html: `<b>Enable JavaScript to run this app.</b>`
          }}
        />
+        <div id="root">{children}</div>
-       {children}
        <script
          dangerouslySetInnerHTML={{
            __html: `assetManifest = ${JSON.stringify(assets)};`
          }}
        />
      </body>
    </html>
  );
}

App.js

export default function App({ assets }) {
  return (
    <Html assets={assets} title="Hello">
+         <AppContent />
-         <Suspense fallback={<Spinner />}>
-            <ErrorBoundary FallbackComponent={Error}>
-                <Content />
-            </ErrorBoundary>
=         </Suspense>
    </Html>
  );
}

+ export function AppContent() {
+  return (
+    <Suspense fallback={<Spinner />}>
+      <ErrorBoundary FallbackComponent={Error}>
+        <Content />
+      </ErrorBoundary>
+    </Suspense>
+  );
+ }

index.js:

- hydrateRoot(document <AppContent />);
+ hydrateRoot(document.getElementById("root"), <AppContent />);

~I don't know if this crashes with Cypress though.~ The app doesn't seem to crash under cypress.

Cypress was adding

function $RC(a,b) {...} 

to the document. I'd assume it would crash if I hydrated the document.

hrgui avatar Apr 25 '22 04:04 hrgui

I don’t think the “stricter” behavior here is intentional. I’ll be checking with the team but my current understanding is that this is a bug.

gaearon avatar Apr 25 '22 12:04 gaearon

Something I've noticed is that when React does encounter a hydration mismatch, it attempts to fallback to client side rendering.

Which does show up in the example codesandbox (the one where we are hydrating the document):

Warning: An error occurred during hydration. The server HTML was replaced with client content in <#document>.
Uncaught Error: There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.

However, it results in an application crash because of the next error:

 Failed to execute 'appendChild' on 'Node': Only one element on document allowed.

which is from the call stack: appendChildToContainer <- insertOrAppendPlacementNodeIntoContainer (3) <- commitPlacement <- commitMutationEffectsOnFiber <- commitMutationEffects_complete <- commitMutationEffects_begin <- commitMutationEffects <- commitRootImpl

Then later another error is thrown:

Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

which is from the call stack: removeChildFromContainer <- unmountHostComponents <- commitDeletion <- commitMutationEffects_begin <- commitMutationEffects <- commitRootImpl <- commitRoot <- performSyncWorkOnRoot <- flushSyncCallbacks <- commitRootImpl


What I am wondering about is: In the case of falling back to client side rendering, why does React do appendChild instead of replaceChild? If it was replaceChild, there wouldn't be an app crash in this case, but at the cost of needing to fall back to client side rendering.

hrgui avatar Apr 30 '22 02:04 hrgui

Yes, that issue is https://github.com/facebook/react/issues/22833. I believe the fix we wanted to do is changing which top-level element we modify. (Maybe body instead of html?) It would be nice to not have to introduce a separate “host config” method (which we’d have to do if we added a call to “replace”). So ideally the fix should use the methods we already use.

gaearon avatar Apr 30 '22 13:04 gaearon

Not an ideal solution but in my use case, I'm only concerned with generating/modifying the head during SSR, and the following hack allows me to work around errors that occur as the result of modifications to the head outside of React by Cypress, various chrome plugins, etc.

const Head: React.FC = () => {

  if (globalThis.document?.head) {
    return (
      <head dangerouslySetInnerHTML={{ __html: document.head.innerHTML }} />
    );
  }

  return (
    <head>
      {/* ... Do important things on the server */}
    </head>
  );
};

This is especially useful for me because even with current fixes added to react-dom@next that allow the client to "recover", doing so wipes out all styles generated by @emotion that have been collected into the head.

rdadoune avatar May 19 '22 21:05 rdadoune

I think I have the same error. But mine is from using styled-components. Initially, they put a style tag in the body, but then the style tag gets moved up to the head. You can check this repo here: https://github.com/adbutterfield/fast-refresh-express/tree/react-18

I tried just now using react@next/react-dom@next, but I still get the error.

Of course, it's always possible that I'm doing something stupid in my code...

adbutterfield avatar Jun 03 '22 02:06 adbutterfield

I tried React 18.2.0, but it still breaks the page when Apollo Client DevTools extension is used. So my ugly solution for this problem is fix the DOM before hydrateRoot:

document.querySelectorAll('html > script').forEach((s) => {
  s.parentNode.removeChild(s);
});

const root = hydrateRoot(
  document,
  <React.StrictMode>
    <AppWithFullDocument />
  </React.StrictMode>,
);

Note: You can replace the selector more strict query .. e.g. 'html > script[src^=chrome-extension]'

(remix-run/remix#2947)

Mordred avatar Jun 15 '22 14:06 Mordred

Getting the issue with Cypress tests, as suggested in Remix Discord, my workaround for those coming here:

if (process.env.NODE_ENV === 'test') {
  require('react-dom').hydrate(<RemixBrowser />, document);
} else {
  hydrateRoot(document, <RemixBrowser />);
}

🙏 for a fix soon, thx all!

dbashford avatar Jun 21 '22 22:06 dbashford

@dbashford Which in particular issue are you hitting? We’ve released a short-term fix for the most urgent part of the problem in 18.2 (document fallback didn’t work at all). Now it should work by doing a full client render but this will lose the third party things. There is a possible fix to do something smarter but it’s a bigger project and a bit further on the timeline. So it would help to know what exactly broke for you. Is there a repro? Particular scenario? Thanks.

gaearon avatar Jun 22 '22 00:06 gaearon

Mine is the Cypress case. Everything works fine in dev and in prod, but Cypress when it kicks up fails (418), and only after making the hydrateRoot switch. I haven't dug into it too far to really understand the problem, but I've read in this thread and in the Remix discord that the problem may be that Cypress monkey's with the head causing the hydration issue. I admit that SSR/hydration are still voodoo/witchcraft to me, so I'm struggling a bit to work my way through it. Baby steps.

dbashford avatar Jun 22 '22 00:06 dbashford

Hey @Mordred thanks a lot for this quick hack, I modified it slightly to also get rid of inputs within html as the Yoroi extensions injects a hidden input directly into the <html/> (not <body/>)

document.querySelectorAll("html > script, html > input").forEach((s) => {
  s.parentNode?.removeChild(s);
});

Using this now in my Remix Deno Stack with Streaming 🥳

CanRau avatar Jun 22 '22 01:06 CanRau

Nevermind this, was local NODE_ENV issue (cypress issue above is still a problem, though)

~~Also having issues with google analytics writing script tags to the head~~

image

~~In this case /blog/gtag pulls in the tag manager snippet...~~

~~Google tag manager starts adding scripts to the head, like google-analytics, and things go sideways from there. I've got microsite where this seems to be working, and some where it isn't.~~

dbashford avatar Jun 22 '22 13:06 dbashford

@dbashford Can you share a minimal project? It always helps to have concrete examples to check fixes against. This goes for everyone else too.

gaearon avatar Jun 22 '22 14:06 gaearon

@gaearon Can call off the dogs on that latest comment, was something I introduced myself while trying to debug a real hydration issue with dates.

The Cypress issue is still a problem, that's all local and consistent. I'll work to get a repro up over the next few weeks when I get a second.

dbashford avatar Jun 22 '22 18:06 dbashford

I just updated my repo with some instructions to more easily reproduce the issue, and test that it works when not using styled components.

See here: https://github.com/adbutterfield/fast-refresh-express/tree/react-18

adbutterfield avatar Jun 24 '22 03:06 adbutterfield

Hi @gaearon I have a repro repo for the hydration errors with cypress. It's not exactly "minimal", it's a freshly-created Remix grunge stack app, which has quite a bit of stuff in it. Here it is: https://github.com/camjackson/remix-cypress-error.

Here are the steps to repro the error locally:

  1. git clone [email protected]:camjackson/remix-cypress-error.git
  2. cd remix-cypress-error
  3. yarn
  4. yarn test:e2e:run

The test will fail with a hydration error. To show that it's related to react 18, you can open up app/entry.client.tsx and comment/un-comment a few lines to switch it from hydrateRoot back to the old hydrate. Then run the e2es again and it will pass.

To debug it further, instead of yarn test:e2e:run you can do yarn test:e2e:dev to fire up cypress in interactive mode and see the test fail in a real browser.

Oh and one final note, if you just start the app normally with yarn dev you should see that there are no errors. It only happens with cypress, presumably because cypress injects something extra into the document.

For completeness, here's how I created this repo from scratch:
  1. yarn create remix --template remix-run/grunge-stack
  2. Go through the prompts, give it a name and cd into the newly created project
  3. yarn add react react-dom
  4. yarn add -D @types/react @types/react-dom
  5. Open up app/entry.client.tsx and migrate from react-dom's hydrate to react-dom/client's hydrateRoot

camjackson avatar Jun 25 '22 03:06 camjackson

I have replicated same behaviour - it can be replicated anywhere with hydrateRoot with any Chrome extension that injects the script. It then switches to client-side rendering.

Uncaught Error: Hydration failed because the initial UI does not match what was rendered on the server.

BleedingDev avatar Jun 26 '22 21:06 BleedingDev

it's also preventing loom (extension) to work as the extension add a to the dom. Seems like it just break any extension that update the dom and having a workaround every potential extension is going to be a nightmare

fix for loom

document.querySelectorAll('loom-container').forEach((s) => {
    s.parentNode.removeChild(s);
});

additionally this also cause warning if the extension is adding attributes like this one with colorzilla image

0Lucifer0 avatar Jun 27 '22 07:06 0Lucifer0

Also causing issue with LastPass, can confirm Loom as well, causing issues across multiple plugins for us.

dbashford avatar Oct 31 '22 22:10 dbashford

Apart from extensions, this issue is affecting libraries that hot load scripts or styles on bootstrapping. In Clerk we use this technique to ensure that users of Clerk-powered applications always have the latest fixes in their authentication components.

A workaround is to delay the hot loading of the components code on the client side. Unfortunately, this is not acceptable for authentication as it will increase the necessary load time before the user is able to interact with the page.

SokratisVidros avatar Nov 01 '22 16:11 SokratisVidros

Still an issue for react apps that render full document. If anyone is using browser extension that injects scripts during the page load, there is an hydration error occuring. Is it safe to use hydrate instead of hydrateRoot, since someone mentioned that it is working with react-dom hydrate as intended.

dejanr avatar Nov 21 '22 10:11 dejanr

Happening with Grammarly and Vimeo Recorder too 😞 If I disable LastPass, and the other two, it's fine.

kamranayub avatar Dec 08 '22 15:12 kamranayub

This problem makes me unable to use Remix framework normally. Although we know that disabling Chrome extensions can solve the problem, we cannot ask our users to disable Chrome extensions. However, many Chrome extensions will inject content into Html documents now, so I wonder when the authorities will pay attention to this problem?

Ryongyon avatar Dec 14 '22 10:12 Ryongyon

@gaearon It looks like the problem appears only when u try to stream html into document because of additional scripts from browser extensions. So when I tried to stream the app in the div#root it really worked fine and solved all hydration issues (because now hydration only work on our div#root which doesn't have any extra browser scripts etc..).

This is the working code example (branch: feat/stream)

denchiklut avatar Dec 26 '22 09:12 denchiklut

I was able to resolve my issue by invoking clearBrowserExtensionInjectionsBeforeHydration() prior to executing hydrateRoot(). https://gist.github.com/OnurGvnc/31f03f0d5237b78224aa083493fda645

OnurGvnc avatar Jan 13 '23 10:01 OnurGvnc

I was able to resolve my issue by invoking clearBrowserExtensionInjectionsBeforeHydration() prior to executing hydrateRoot(). https://gist.github.com/OnurGvnc/31f03f0d5237b78224aa083493fda645

Yeah it's not really a good fix as it just remove all the potential dom changes... Still useful as a temp fix until better solution... not sure why this bug is still tagged as unconfirmed after 9 month when it's literally making react18 not production ready

0Lucifer0 avatar Jan 13 '23 11:01 0Lucifer0

Hi, still having issues with this ... I saw/read a lot of issues and did not see a universal solution (if there is one) ... maybe someone can help me with an answer? Also on a side note... not sure it is related but I have CSS flickers and not sure if is related to this or not.

Thanks.

Mihai-github avatar Jan 13 '23 11:01 Mihai-github

Hi, still having issues with this ... I saw/read a lot of issues and did not see a universal solution (if there is one) ... maybe someone can help me with an answer? Also on a side note... not sure it is related but I have CSS flickers and not sure if is related to this or not.

Thanks.

Same for the css flickers not sure why. I don’t think it’s related as I didn’t use to have it it appeared recently

erwan-joly avatar Jan 13 '23 11:01 erwan-joly