search

remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
30.01k stars 2.53k forks source link

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

Closed hrgui closed 2 years ago

hrgui commented 2 years ago

What version of Remix are you using?

the main branch of remix (hash a759affafe30c8df27e36f6253a1d9a574ebf18d)

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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: https://github.com/facebook/react/issues/22833, https://github.com/facebook/react/issues/24430

In my non-remix app, I had to do this fix: https://github.com/hrgui/nodejs-react-sc-demo/commit/ac872341d5e2c93d80c562a0f4056e2ae757c7da.

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.

DAlperin commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Cross linking this: https://github.com/facebook/react/issues/22833

Aprillion commented 2 years ago

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 https://github.com/darkreader/darkreader/issues/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?

jacob-ebey commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

From my understanding, 18.2.0-next-a412d787e-20220518 contains the fix https://github.com/facebook/react/pull/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 https://github.com/reactjs/reactjs.org/issues/4656#issuecomment-1126180807, I believe the react team is working on a bigger plan to make this work on a more resilient way.

huw commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

React 18.2.0 is latest as of June 14 which includes https://github.com/facebook/react/pull/24523

aaacafe786 commented 2 years ago

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 commented 2 years ago

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

bangngo52 commented 2 years ago

I have the same experience with @cjoecker when removing the Githubissues.

  • Githubissues is a development platform for aggregating issues.