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

[React 18] hydrateRoot(document, <RemixBrowser />) causes app crash with any scripts that modified DOM before hydration #2947

Closed
hrgui opened this issue Apr 21, 2022 · 23 comments

Comments

@hrgui
Copy link

hrgui commented Apr 21, 2022

What version of Remix are you using?

the main branch of remix (hash a759aff)

Steps to Reproduce

Install the Dark Reader Chrome Extension: https://chrome.google.com/webstore/detail/dark-reader/eimadpbcbfnmbkopoojfekhnkhdbieeh?hl=en-US

  1. Checkout the remix repo (this repo.)
  2. Run yarn playground:new
  3. After finishing, run yarn dev in the newly created playground folder, yarn watch in the root of remix

Expected Behavior

should see Remix Playground, Sign up / Log In buttons with Dark Reader able to run

Actual Behavior

The playground loads for a sec, then when React does hydrateRoot, it fails to load if the Dark Reader Chrome Extension is enabled.

The workaround is to don't use any chrome extensions that modify the DOM in order to run the playground.

react-dom.development.js:86 Warning: Expected server HTML to contain a matching <meta> in <head>.
    at meta
    at Meta (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:2816:7)
    at head
    at html
    at App
    at RemixRoute (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:2586:3)
    at Routes2 (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:2569:7)
    at Router (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:679:15)
    at RemixCatchBoundary (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:1079:10)
    at RemixErrorBoundary (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:1004:5)
    at RemixEntry (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:2466:12)
    at RemixBrowser (http://localhost:3000/build/_shared/chunk-EYMZ6H3Z.js:3213:27)
printWarning @ react-dom.development.js:86
5react-dom.development.js:14344 Uncaught Error: Hydration failed because the initial UI does not match what was rendered on the server.
    at throwOnHydrationMismatch (react-dom.development.js:14344:9)
    at tryToClaimNextHydratableInstance (react-dom.development.js:14372:7)
    at updateHostComponent$1 (react-dom.development.js:20636:5)
    at beginWork (react-dom.development.js:22373:14)
    at HTMLUnknownElement.callCallback2 (react-dom.development.js:4157:14)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:4206:16)
    at invokeGuardedCallback (react-dom.development.js:4270:31)
    at beginWork$1 (react-dom.development.js:27243:7)
    at performUnitOfWork (react-dom.development.js:26392:12)
    at workLoopSync (react-dom.development.js:26303:5)

react-dom.development.js:11996 Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
    at removeChildFromContainer (http://localhost:3000/build/entry.client-R32SQO3U.js:7970:23)
    at unmountHostComponents (http://localhost:3000/build/entry.client-R32SQO3U.js:16770:17)
    at commitDeletion (http://localhost:3000/build/entry.client-R32SQO3U.js:16814:13)
    at commitMutationEffects_begin (http://localhost:3000/build/entry.client-R32SQO3U.js:16958:19)
    at commitMutationEffects (http://localhost:3000/build/entry.client-R32SQO3U.js:16946:11)
    at commitRootImpl (http://localhost:3000/build/entry.client-R32SQO3U.js:18515:13)
    at commitRoot (http://localhost:3000/build/entry.client-R32SQO3U.js:18446:13)
    at performSyncWorkOnRoot (http://localhost:3000/build/entry.client-R32SQO3U.js:18067:11)
    at flushSyncCallbacks (http://localhost:3000/build/entry.client-R32SQO3U.js:8615:30)
    at commitRootImpl (http://localhost:3000/build/entry.client-R32SQO3U.js:18590:11)
@hrgui hrgui changed the title Running Dark Reader Chrome Extension causes the remix playground to crash [React 18] Running Dark Reader Chrome Extension causes the remix playground to crash Apr 21, 2022
@hrgui
Copy link
Author

hrgui commented Apr 21, 2022

Note:

Upgrading react-dom/react to v18 and changing hydrate to hydrateRoot is also affected by this bug. I've provided a test repo to use: https://github.com/hrgui/react-18-remix-app

When doing the following changes:

package.json:

- "react": "^17.0.2",    
+ "react": "^18.0.0",
- "react-dom": "^17.0.2",    
+ "react-dom": "^18.0.0"

entry.client.jsx:

import { RemixBrowser } from "@remix-run/react";
- import { hydrate } from "react-dom";
+ import { hydrateRoot } from "react-dom/client";

- hydrate(<RemixBrowser />, document);
+ hydrateRoot(document, <RemixBrowser />);

@DAlperin
Copy link

I deployed my first production remix app and have gotten consistent reports from users who have any extension that mutated the DOM (many many of them) I reverted to react 17, but this issue should be considered 'breaking'.

We should probably retitle this issue to reflect that it's not just dark reader, but any extension that injects script tags or mutates the DOM.

@ksavblik
Copy link

ksavblik commented Apr 24, 2022

Having the same issue, and agree with @DAlperin for renaming this one
So the real problem is not the mismatch itself, but in the unrecoverable error from new hydrateRoot(in React 17 hydrate just warns about the mismatch)
This is actually relevant for other frameworks as well(that renderhydrate the whole document), not only remix
react issues: facebook/react#22833, facebook/react#24430
The question is whether this behavior is expected or not

@hrgui hrgui changed the title [React 18] Running Dark Reader Chrome Extension causes the remix playground to crash [React 18] hydrateRoot(document, <RemixBrowser />) crashes with any scripts that modified DOM before hydration Apr 25, 2022
@hrgui
Copy link
Author

hrgui commented Apr 25, 2022

We should probably retitle this issue to reflect that it's not just dark reader, but any extension that injects script tags or mutates the DOM.

Changed the title to reflect the true issue. This also applies for this tweet https://twitter.com/ebey_jacob/status/1509666902477402135. The app doesn't work until I disable apollo devtools / dark reader.

This is actually relevant for other frameworks as well(that render the whole document), not only remix
react issues: facebook/react#22833, facebook/react#24430

In my non-remix app, I had to do this fix: hrgui/nodejs-react-sc-demo@ac87234.

  • The server renders the entire document
  • The browser only hydrates the root div element.

The question is, how do we make this work with <RemixBrowser /> ? I tried the following:

hydrateRoot(document.body, <RemixBrowser />);

^ this doesn't crash, but it causes the app to fall back to client side rendering. This is because there is obviously a hydration mismatch - <RemixBrowser /> is the entire document.

hydrateRoot(document.getElementById('root'), <RemixBrowser />);

^ this doesn't crash, but same issue - it causes the app to fall back to client side rendering. This is because there is obviously a hydration mismatch - <RemixBrowser /> is the entire document.

hydrateRoot(document.getElementById('root'), <Outlet />);

^ This doesn't present any errors, but client-side hydration doesn't work as a simple counter does not seem to increment.

@hrgui hrgui changed the title [React 18] hydrateRoot(document, <RemixBrowser />) crashes with any scripts that modified DOM before hydration [React 18] hydrateRoot(document, <RemixBrowser />) causes app crash with any scripts that modified DOM before hydration Apr 25, 2022
@DAlperin
Copy link

The question is, how do we make this work with ? I tried the following:

The RemixBrowser API needs a new surface for this use case, I think. I can take a crack at a PR this week if I have time. It would be nice to hear from the core team on how they would like to approach this before I try anything though.

Cc: @mjackson @ryanflorence

@DAlperin
Copy link

DAlperin commented Apr 25, 2022

Though, on second thought not being able to hydrate the <head> content would break a lot of things. So I'm not sure that this is the right tradeoff.

@jacob-ebey
Copy link
Member

Cross linking this: facebook/react#22833

@Aprillion
Copy link
Contributor

Aprillion commented May 7, 2022

I got reports that my remix site stopped working after update to React 18 from users with browser plugins which add <meta> or <script> tags, e.g.:

Dark Reader: https://chrome.google.com/webstore/detail/dark-reader/eimadpbcbfnmbkopoojfekhnkhdbieeh
=> reported in darkreader/darkreader#8842

Express VPN: https://chrome.google.com/webstore/detail/expressvpn-vpn-proxy-for/fgddmllnllkalaagkghckoinaemmogpe
=> no idea why VPN would be a browser plugin, I don't care enough to report to them

Does anyone know of a better workaround than reverting to React 17 until this issue is solved by either React or Remix team?

Aprillion added a commit to StampyAI/stampy-ui that referenced this issue May 7, 2022
ostwilkens added a commit to ostwilkens/remix-minimal-stack that referenced this issue May 8, 2022
Ponjimon added a commit to Ponjimon/remix-cloudflare-pages-template that referenced this issue May 14, 2022
@jacob-ebey
Copy link
Member

I believe the React team will have a fix for this in the next release. The 18.2.0-next-a412d787e-20220518 release seems to address the underlying issue.

@hrgui
Copy link
Author

hrgui commented May 19, 2022

@jacob-ebey I just created a codesandbox with 18.2.0-next-a412d787e-20220518 with the react SSR example.

https://codesandbox.io/s/muddy-worker-t9g9ud
https://t9g9ud.sse.codesandbox.io/

It no longer crashes the application, which is good. However, the entire application still has to fallback to client-side rendering, which I believe may cause some unintended issues. In the app that I have above the application had fully rendered, then everything is rerendering when hydration kicks in.

@hrgui
Copy link
Author

hrgui commented May 19, 2022

From my understanding, 18.2.0-next-a412d787e-20220518 contains the fix facebook/react#24523. The expectations set there is: yes, there will be hydration mismatches, but the client will fallback.

In that case, the example in https://t9g9ud.sse.codesandbox.io/ is working as per the react team intended for now.

As per reactjs/react.dev#4656 (comment), I believe the react team is working on a bigger plan to make this work on a more resilient way.

@huw
Copy link
Contributor

huw commented May 20, 2022

Does React 18 allow Remix to create multiple roots for different parts of the document? If that were the case, a potential solution could be to have a <head> root and a separate <body> root, which would obviate most browser extension problems AFAIK

@ryanyogan
Copy link

ryanyogan commented Jun 12, 2022

If you upgrade to https://www.npmjs.com/package/react/v/18.2.0-next-a412d787e-20220518 you can do the following:

In your entry.client.tsx file

import { RemixBrowser } from "@remix-run/react";
import { hydrateRoot } from "react-dom/client";

hydrateRoot(document, <RemixBrowser />);

In your entry.server.tsx file

import { PassThrough } from "stream";
import { renderToPipeableStream } from "react-dom/server";
import { RemixServer } from "@remix-run/react";
import { Response } from "@remix-run/node";
import type { EntryContext, Headers } from "@remix-run/node";
import isbot from "isbot";

const ABORT_DELAY = 5000;

export default function handleRequest(
  request: Request,
  responseStatusCode: number,
  responseHeaders: Headers,
  remixContext: EntryContext
) {
  const callbackName = isbot(request.headers.get("user-agent"))
    ? "onAllReady"
    : "onShellReady";

  return new Promise((resolve, reject) => {
    let didError = false;

    const { pipe, abort } = renderToPipeableStream(
      <RemixServer context={remixContext} url={request.url} />,
      {
        [callbackName]() {
          let body = new PassThrough();

          responseHeaders.set("Content-Type", "text/html");

          resolve(
            new Response(body, {
              status: didError ? 500 : responseStatusCode,
              headers: responseHeaders,
            })
          );
          pipe(body);
        },
        onShellError(err: unknown) {
          reject(err);
        },
        onError(error: unknown) {
          didError = true;
          console.error(error);
        },
      }
    );
    setTimeout(abort, ABORT_DELAY);
  });
}

And you should be streaming from the server again on the initial render.

This is just a short-term hack; I pulled this from Kent Dodds's upcoming Front End Masters course, seems like an awesome one!

@knpwrs
Copy link

knpwrs commented Jun 23, 2022

React 18.2.0 is latest as of June 14 which includes facebook/react#24523

@aaacafe786
Copy link

aaacafe786 commented Jul 26, 2022

I would like to bump this issue, we have upgraded our stack to the latest version of react (18.2.0), we modified entry.server (https://github.com/remix-run/indie-stack/blob/main/app/entry.server.tsx) and entry.client (https://github.com/remix-run/indie-stack/blob/main/app/entry.client.tsx) inline with the latest version of the indie stack.

After deploying to production, we are finding errors:
image

If you would like to see a reproduction of the issue, please see here: https://pr-30-osc-academic-hub.fly.dev/

Obviously this diverges slightly from the original issue, as this screenshot demonstrates the error in incognito mode with no extensions installed.

edit -> after doing some further investigation, we have pinpointed one of the extensions causing the issue (loom, screen recorder)

@cjoecker
Copy link

cjoecker commented Aug 2, 2022

I realice that the issue comes from the <Scripts /> component. But still I'm not sure why.

@bangngo52
Copy link

I have the same experience with @cjoecker when removing the <Script /> the message is disappeared

@CanRau
Copy link
Contributor

CanRau commented Aug 11, 2022

That's because without <Scripts/> you're site is completely static, no JavaScript, no hydration and therefore no client hydration, so no hydration errors

@dr3
Copy link

dr3 commented Aug 18, 2022

@aaacafe786 It appears the

Warning: Did not expect server HTML to contain a <script> in <html>.

issue is a documented "gotcha" https://remix.run/docs/en/v1/pages/gotchas#browser-extensions-injecting-code

With the solution being "Check out the page in incognito mode, the warning should disappear."
Granted this feels like a very much a non-solution, as the error happens in production, and affects any user who has certain (popular) chrome extensions installed

@hrgui Should this issue be closed now (primary issue was fixed in react 18.2.0), and we can open a new issue to track the "gotcha"?

@hrgui
Copy link
Author

hrgui commented Aug 18, 2022

Check out the page in incognito mode, the warning should disappear.

Note that users can still use extensions in incognito if they desire (just need to check the checkbox)

To be fair, it looks like the original problem has been solved as the application no longer crashes. However, the application resorts to client-side rendering as of React 18.2.0 in the case when an injection happens from an extension.

@dr3 Is there a new issue already opened for the gotcha? Once that new issue is opened, then I don't mind closing this issue as solved and encouraging folks to move on to the gotcha issue.

@dr3
Copy link

dr3 commented Aug 18, 2022

I have since found this comment from @kentcdodds where he says the issue isn't something that remix can fix.

@hrgui I see you commented on this issue in the react repo which seems to be the same issue, so I think we can probably close this issue, and hope that this is fixed via the react issue

@hrgui hrgui closed this as completed Aug 18, 2022
@hrgui
Copy link
Author

hrgui commented Aug 18, 2022

Sounds good @dr3, I have closed this issue as completed.

The original problem of an application crash has been fixed in React 18.2.0.

Users that have chrome extensions that do inject into the DOM will encounter a client-side re-render warning. In Cypress it will be treated as an error since React uses error for that. Follow facebook/react#24430.

I believe the only remaining action the Remix team could possibly do is not hydrate the entire document in order to fix the problem. Otherwise, we are waiting on facebook/react#24430.

SokratisVidros added a commit to clerk/javascript that referenced this issue Nov 2, 2022
Store frontendAPI value on window as a fallback. This value can be used as a fallback during ClerkJS hot loading in case ClerkJS fails to find the "data-clerk-frontend-api" attribute on its script tag.

This can happen when the DOM is altered completely during client rehydration.
For example, in Remix with React 18 the document changes completely via `hydrateRoot(document)`.

For more information refer to:

- remix-run/remix#2947
- facebook/react#24430
SokratisVidros added a commit to clerk/javascript that referenced this issue Nov 2, 2022
Store frontendAPI value on window as a fallback. This value can be used as a fallback during ClerkJS hot loading in case ClerkJS fails to find the "data-clerk-frontend-api" attribute on its script tag.

This can happen when the DOM is altered completely during client rehydration.
For example, in Remix with React 18 the document changes completely via `hydrateRoot(document)`.

For more information refer to:

- remix-run/remix#2947
- facebook/react#24430
@iamvijaydev
Copy link

Hi, a dev from 2024, who also faced the same issue with Cloudflare Pages template. I tested it in a variety of configurations. Even with all the extensions disabled. As someone pointed out above (/previously closed thread) the core problem is with react@18.2.

After trying a few React builds the following canary build solved the issue. This also worked with plugins like Grammarly, which we know changes the DOM before hydration.

- "react": "^18.2.0",
- "react-dom": "^18.2.0"
+ "react": "19.0.0-canary-33a32441e9-20240418",
+ "react-dom": "19.0.0-canary-33a32441e9-20240418"

I hope this will help out there anyone unstuck and continue to use the awsome features of Remix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

15 participants