Rendering all breakpoints on the server and then relying on hydration fixup to prune them is too expensive in 18

[gurkerl83]

We use a library called Fresnel to achieve the following

1. Render markup for all breakpoints on the server and send it down the wire.
2. The browser receives markup with proper media query styling and will immediately start rendering the expected visual result for whatever viewport width the browser is at.
3. When all JS has loaded and React starts the rehydration phase, we query the browser for what breakpoint it’s currently at and then limit the rendered components to the matching media queries. This prevents life-cycle methods from firing in hidden components and unused html being re-written to the DOM.

Latest compatible version: 18.0.0-rc.0-next-fa816be7f-20220128
First incompatible version: 18.0.0-rc.0-next-3a4462129-20220201
Most recent versions are still incompatible

Identified changes short after the last compatible version was published.

I did some digging into the recent changes in React and may have been able to identify the problem.

The initial report artsy/fresnel#260 (comment) describes an error thrown when server-side generated components no longer match those on the client-side. This change of application behavior was introduced in the following commit.

3f5ff16

In the same commit, further changes are made, with at least the following leading to another problem (assuming error throwing is disabled).

react/packages/react-reconciler/src/ReactFiberHydrationContext.old.js

Line 563 in 3f5ff16

} else {

if (nextInstance) {
  if (shouldClientRenderOnMismatch(fiber)) {
   warnIfUnhydratedTailNodes(fiber);
   throwOnHydrationMismatchIfConcurrentMode(fiber); // => (*2)
  }
  else { // => (*1)
   while (nextInstance) {
     deleteHydratableInstance(fiber, nextInstance);
     nextInstance = getNextHydratableSibling(nextInstance);
   }
  }
}

Local tests show that the condition statement "if/else" block is wrong; the delete operation must always be executed.

// *1 The delete operation of unmatched siblings needs to be called anyway; otherwise, DOM and React get out of sync, meaning phantom DOM entries (duplicated DOM Elements) get generated when re-rendering occurs. Those elements do not have a corresponding react component in the dev tools.

// *2 Throwing errors have to be optional, not mandatory, options to think about

Remove throwing errors altogether; at least make it optional because the third argument in hydrateRoot is not used/implemented by any consumer of this API, such as in NextJS, although they promise you can use the latest experimental React version
Disable enableClientRenderFallbackOnHydrationMismatch when suppressHydrationWarning is set.

Note: When looking at the associated hydration test suite https://github.com/facebook/react/blob/main/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js it is noticeable that in the tests mostly suspense is used. In the following test (no test exclusively output after hydration, first and second render run) simple elements are used.

it('with if / else in place', async () => {
  function App({hasA, hasB}) {
    return (
      <div>
        <div>{hasA ? <span>A</span> : null}</div>
        <div>{hasB ? <span>B</span> : null}</div>
      </div>
    );
  }

  const finalHTML = ReactDOMServer.renderToString(
    <App hasA={true} hasB={true} />, // Render markup for all breakpoints on the server and send it down the wire.
  );

  const container = document.createElement('div');
  container.innerHTML = finalHTML;

  const root = ReactDOM.hydrateRoot(
    container,
    <App hasA={true} hasB={false} />,
  );

  jest.runAllTimers();
  Scheduler.unstable_flushAll();

  /**
   * Results:
   * 1. current version if / else in place (wrong): => <div><div><span>A</span></div><div><span>B</span></div></div>
   * 2. only one if - delete on default (expected): <div><div><span>A</span></div><div></div></div>
   */
  console.log(
    'after hydration / hasA={true} hasB={false}:',
    container.innerHTML,
  );

  root.render(<App hasA={false} hasB={true} />);

  jest.runAllTimers();
  Scheduler.unstable_flushAll();

  /**
   * Results:
   * 1. current version if / else in place (wrong - phantom elements created): => <div><div></div><div><span>B</span><span>B</span></div></div>
   * 2. only one if - delete on default (expected): <div><div></div><div><span>B</span></div></div>
   */
  console.log(
    'first re-render / hasA={false} hasB={true}:',
    container.innerHTML,
  );

  root.render(<App hasA={true} hasB={false} />);

  jest.runAllTimers();
  Scheduler.unstable_flushAll();

  /**
   * Results:
   * 1. current version if / else in place (wrong): <div><div><span>A</span></div><div><span>B</span></div></div>
   * 2. only one if - delete on default (expected): <div><div><span>A</span></div><div></div></div>
   */
  console.log(
    'second re-render / hasA={true} hasB={false}:',
    container.innerHTML,
  );
});

Maybe you guys can give some feedback if the identified problem in those changes made is really the cause to the problem.

Thx!

[eps1lon]

How does this approach work in React 17? If it creates hydration warnings in React 17 then it's expected to also cause problems in React 18.

including wild error throwings, out of sync DOM nodes

Could you specify which errors are thrown?

Ideally you could create a repro by forking https://codesandbox.io/s/kind-sammet-j56ro

[salazarm]

// *1 The delete operation of unmatched siblings needs to be called anyway; otherwise, DOM and React get out of sync, meaning phantom DOM entries (duplicated DOM Elements) get generated when re-rendering occurs. Those elements do not have a corresponding react component in the dev tools.

The delete operation doesn't need to be called here because we end up calling clearContainer after setting Snapshot flag on the fiber

// *2 Throwing errors have to be optional, not mandatory, options to think about

Remove throwing errors altogether; at least make it optional because the third argument in hydrateRoot is not used/implemented by any consumer of this API, such as in NextJS, although they promise you can use the latest experimental React version Disable enableClientRenderFallbackOnHydrationMismatch when suppressHydrationWarning is set.

That error is thrown as an internal control mechanism. It's caught here . Only a legitimate error would bubble up.

Note: When looking at the associated hydration test suite https://github.com/facebook/react/blob/main/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js it is noticeable that in the tests mostly suspense is used. In the following test (no test exclusively output after hydration, first and second render run) simple elements are used.

it('with if / else in place', async () => {
  function App({hasA, hasB}) {
    return (
      <div>
        <div>{hasA ? <span>A</span> : null}</div>
        <div>{hasB ? <span>B</span> : null}</div>
      </div>
    );
  }

  const finalHTML = ReactDOMServer.renderToString(
    <App hasA={true} hasB={true} />, // Render markup for all breakpoints on the server and send it down the wire.
  );

  const container = document.createElement('div');
  container.innerHTML = finalHTML;

  const root = ReactDOM.hydrateRoot(
    container,
    <App hasA={true} hasB={false} />,
  );

  jest.runAllTimers();
  Scheduler.unstable_flushAll();

  /**
   * Results:
   * 1. current version if / else in place (wrong): => <div><div><span>A</span></div><div><span>B</span></div></div>
   * 2. only one if - delete on default (expected): <div><div><span>A</span></div><div></div></div>
   */
  console.log(
    'after hydration / hasA={true} hasB={false}:',
    container.innerHTML,
  );

  root.render(<App hasA={false} hasB={true} />);

  jest.runAllTimers();
  Scheduler.unstable_flushAll();

  /**
   * Results:
   * 1. current version if / else in place (wrong - phantom elements created): => <div><div></div><div><span>B</span><span>B</span></div></div>
   * 2. only one if - delete on default (expected): <div><div></div><div><span>B</span></div></div>
   */
  console.log(
    'first re-render / hasA={false} hasB={true}:',
    container.innerHTML,
  );

  root.render(<App hasA={true} hasB={false} />);

  jest.runAllTimers();
  Scheduler.unstable_flushAll();

  /**
   * Results:
   * 1. current version if / else in place (wrong): <div><div><span>A</span></div><div><span>B</span></div></div>
   * 2. only one if - delete on default (expected): <div><div><span>A</span></div><div></div></div>
   */
  console.log(
    'second re-render / hasA={true} hasB={false}:',
    container.innerHTML,
  );
});

I ran your test against the current version and got all the expected results.

[gurkerl83]

@salazarm
You are right; I got the results in the test when all throwOnHydrationMismatch calls were enabled again in the context file. It seems an error has to be thrown to get the intended results.

A few other questions remain. A Suspense boundary around the content is necessary to avoid bubbling the error up to the root where the app mounts, like in the test suite I mentioned, right?

Another question is towards an Error-Boundary. Do I need one, and where (at the root), you said it is handled internally, here looking a the function body the error seems to re-thrown here or do I mis-interpret this.

One last question is towards backward compatibility, using hydrateRoot without any option and, more specifically, "onRecoverableError." As mentioned above, in most cases, as an application developer, you rely on, e.g., on NextJs or similar, which call hydrateRoot here but for now, with any options specified. Does this new apparatus make it mandatory to specify the hydrateRoot option, or does it work without specifying?

Many thanks in advance!

[salazarm]

A few other questions remain. A Suspense boundary around the content is necessary to avoid bubbling the error up to the root where the app mounts, like in the test suite I mentioned, right?

Originally at the time of that commit yes, but that's because we were delaying the change to make it work without a boundary. Now it should work without a boundary, let me know if you see otherwise because that would be a bug.

Another question is towards an Error-Boundary. Do I need one, and where (at the root), you said it is handled internally, here looking a the function body the error seems to re-thrown here or do I mis-interpret this.

This error shouldn't be rethrown. Do you have a repro of that behavior?

One last question is towards backward compatibility, using hydrateRoot without any option and, more specifically, "onRecoverableError." As mentioned above, in most cases, as an application developer, you rely on, e.g., on NextJs or similar, which call hydrateRoot here but for now, with any options specified. Does this new apparatus make it mandatory to specify the hydrateRoot option, or does it work without specifying?

NextJS would need to add the option if you would like to handle the error with your own javascript (eg. for reporting through Sentry). But even without specifying the option you should the errors in your console.

Also, what is the exact issue you're facing?

[gurkerl83]

@salazarm Thanks for answering my questions, I will make a reproducer in the form of a minimal NextJs app.

[gurkerl83]

@salazarm I have created the reproducer, the sources are here https://github.com/gurkerl83/React-18-Hydration-Bug.git

The latest experimental version (a few hours old) of React gets used in the main branch.
There is also a branch that runs the app without errors on the last working experimental version 18.0.0-rc.0-next-fa816be7f-20220128 - about a month old).

The branch's name is With-Latest-Working-Experimental-Version.
It seems Vercel / NextJs I can not deploy a dev build; only production builds, it is not really helpful, so you have to build locally.

Steps to build and run

1. Clone the repo

2. Install dependencies (just run the command yarn in the folder)
yarn
3. Run development respectively production (either the first or the later for prod builds)
yarn dev
yarn start

4. Open Browser at http://localhost:3000

4. See the error thrown respectively the dev console

The link to the deployed version (build from main branch)
https://react-18-hydration-bug-with-latest-experimental-gurkerl83.vercel.app

Thx!

[salazarm]

Can you use codesandbox please. Also what is the expected behavior

[gurkerl83]

@salazarm I provided those information requested. I hope the wording is not too brash, I actually know little React internals just want to support here.

Here are the sandboxes:

Working version (from a secondary branch in the repo, with Reacts last working experimental version 18.0.0-rc.0-next-fa816be7f-20220128 - about a month old)
https://codesandbox.io/s/dawn-meadow-77t84i

Buggy version (from repos main branch, with Reacts latest experimental version, a few hours old)
https://codesandbox.io/s/sad-bell-16ucyf

Explanation of the app
You can see only 5 divs and their children, but in fact 10 divs are generated by the server and sent to the client, 5 for small screen widths, 5 for larger ones.

In the hydration phase, the children of such divs have to be removed, which do currently not fit; the parents remain in the DOM but are not displayed. Based on the current viewport, one or the other group gets selected.

The removal gets initiated by the client; only the client knows its actual viewport, see https://github.com/gurkerl83/React-18-Hydration-Bug/blob/a3046deca9a225345a3ee26b8cd05b99053d61eb/components/Segment.tsx#L54

When you open the working sandbox demo in a new window, also dev tools with the element inspector and resize the children respond.

Expected behavior

• Do not show the error messages from the buggy demo in the buggy version
• Client-side render fallback gets initiates differently now; see further down. Ensure the performance is even with the older implementation. I mention this here because I imagine invoking clear-container has to be part of some initialization phase here.

Exception throwing techniques to communicate deletion respectively initiate a client render fallback

Although using exception throwing techniques as discussed this afternoon will produce the desired results in testing, exceptions executed fast, one after the other, could cause a problem.

You mentioned the clear-container call gets executed after setting Snapshot in a fiber.

Logging when the clear-container call gets executed respectively, the container info surprised me; it is the entire DOM, not just the fragment container a sibling is part of and has to be removed from, thus invoking a client-side render on this container.

In terms of the demo, this would happen five times; for each mismatch determined, resetting in such a manner would not be very scalable and efficient; imagine removing possible hundreds of elements.

Hopefully, the intention is clear why we pre-render on the server side so many elements, which we then remove on the client again. If not, please look at Fresnel

The old implementation does this without even calling clear-container, I tried it, removed all throws again, and the clear-container function gets not invoked.

If you need more information, please let me know!

Thx!

[salazarm]

I took a look at the sandbox and this is the new expected behavior. We changed to this strategy for consistency reasons because there are edge cases with the old strategy where patching up the DOM would lead to wrong results. I recommend you make the initial render while hydrating return the same elements as the server and only hide the elements by changing attributes on the nodes.

[gurkerl83]

I recommend you make the initial render while hydrating return the same elements as the server and only hide the elements by changing attributes on the nodes.

This is not possible. It would be possible if relevant components differentiate in their styles only. When a swappable component set (mobile and desktop variant) differs in logic or in consuming different APIs, just "hiding" things means that the internals of both components gets executed all the time.

A mobile variant of a component fetches data from API A. The desktop variant fetches data from API A and B, "hiding" means that the fetch of API A gets executed twice, and the fetch fetches execution of API B one time.

Others effects from this strategy are that children of both component variants render all the time; this is also undoubtedly unnecessary overhead.

All these effects will happen on the initial render.
To avoid those problems, conditional render in the individual component variations is required.

See desktop and mobile.

I suspect many use cases will break when the current strategy goes into a stable release. Most important is a fallback towards what you call DOM patching.

The hydration strategy should respect situations where the server does not have all the necessary information in advance. Hydration and, more specifically, the first render of a component has to respect the information available by the client, which influences the output.

When hydration does not apply a fine-grained resolvent strategy at a container level, but on the mounting point, problems such as cumulative layout shift may not be preventable.

API wise, it would be nice to have an option on a container wrapping children similar to the property suppressHydrationWarning, but skipping the process of client render and fallback to the currently stable behavior.

It is just an early measure when conditional render gets executed based on the current media query. I think this has to be respected.

It would also be nice to learn about the edge cases and constellations the development team encountered in the past, which led to this kind of adjustment.

Thx!

[salazarm]

It would also be nice to learn about the edge cases and constellations the development team encountered in the past, which led to this kind of adjustment.

A concrete example that would help you understand the behavior is imagine you're rendering a grid of People and when you click on the person a chat tab to talk to them opens up:

The server sends:

<PersonA>
<PersonB>
<PersonC>

But on the client we end up rendering

<PersonA>
<PersonC>
<PersonB>

function Person({name, openChatTab, id}) {
   return <div onClick={() => openChatTab(id)}>{name}</div>
}

The hydration mismatch would occur on the text name. If we were to just patch this up then we when you click on Person B you'd actually end up talking to Person C. To render this correctly we would need to client render the whole list.

[salazarm]

Actually that example isn't even that bad^ because you could just only client render PersonC and PersonB

This was the example that convinced me:

function Person({name, picUrl, openChatTab, id}) {
  return (
    <div onClick={() => openChatTab(id)}>
       <div><ProfilePicture picUrl={picUrl}/></div>
       <div><ProfileName name={name}/></div>
       <div><UserStatus id={id} />
     </div>
  );
}

In this case the mismatch would occur within ProfilePicture (on the img src attribute) and inside ProfileName (on the text name). If we were to only client-render ProfilePicture and ProfileName then they'd still be attached to the wrong Person component. The real issue is that there's a props/context mismatch on the Person component, but serializing that down to the client and detecting it would kill performance. Maybe we can do that in DEV so that we can warn in that case

[gaearon]

Relevant RFC if you'd like to voice this feedback there as well: reactjs/rfcs#215

In general, rendering different things as a strategy was never supported. Before React 16 it didn't work at all. After React 16, it worked with an asterisk but it was always considered a bug in user code that should be fixed. The documentation said that as well.

The problem with the approach you're using is that it's very unreliable and can easily end up in confusing "mixed" states where some attributes match and some don't. In the worst case, this can lead to privacy/security holes. (E.g. imagine two inputs from different layouts of a Settings screen accidentally "matching up" during hydration, except one of them is a password field. Or imagine a chat list that shows people in a different order on the client and the server, and attaches one person's event handlers to another person's avatar or name, like Marco mentioned above.)

The "supported" way to render different things was to do two-pass rendering by setting state in an effect. So what you could do is serialize which layout was rendered by the server together with the HTML. Then, on the client, make the initial render use that layout (instead of reading it from the client device). And then immediately do a re-render with the actual true layout. This would do more rendering work, but would guarantee a consistent result that doesn't suffer from hydration bugs.

[KrustyC]

@gurkerl83 Are you still facing the same issue, or did you manage to overcome it? I am encountering the same exact problem with Nextjs and Fresnel, and it started to happen as soon as I updated to React 18, did you find a way around it?

[gaearon]

@KrustyC Did you see my response above? I’d like to understand if you’ve tried the strategy I explained there and what exactly is breaking for you.

[gurkerl83]

@gaearon Thanks for the provided hint, an example of how best to do this would be great.

So what you could do is serialize which layout was rendered by the server together with the HTML. Then, on the client, make the initial render use that layout (instead of reading it from the client device). And then immediately do a re-render with the actual true layout.

[gaearon]

You could place a context provider at the top with a boolean state value like canUseMedia. It's false on the server and also false during hydration. Your media query component should read that context. When it's false, your media query component should render whatever the "default" layout you want (regardless of the actual media query it reads and subscribes to). Then, in an effect, change the top-level state you pass to that context provider to true. Your media query component should return the real value if that context is true. This will force a client re-render, and the parts that don't match will be replaced by regular React logic, as when the media query actually changes.

[KrustyC]

@gaearon thanks for the reply. I read your comment but I am afraid that by doing so, the user could experience some weird Layout Shift during the first render. For example, in our app, we have a carousel right at the top of the page which appears completely different in the client and in the server. if I do what you suggest, then I should have a default version, right? Irrespective of what the user device is, so then on the server I would render the "mobile" carousel (for example), but then when the page load, the MediaProvider will start working and then rendering the "desktop" carousel, which will cause a layout shift. We had this problem initially and that is the reason why we started using Fresnel.

Am I missing something about your eplaination?

[gaearon]

I see, when I jumped into the thread I was responding to the last posts and missed the initial explanation of what Fresnel attempts to do. I need to read a bit more to understand how it's working now.

[KrustyC]

that's amazing, thanks! I will also dig a bit more to see if I can find a work around maybe

[gaearon]

OK, if Fresnel renders all breakpoints on the server, how about:

1. Keep rendering all of them on the server
2. Also ensure all of them are rendered on the client during hydration (but hidden with CSS)
3. Set context after hydration (in an effect) that tells the Media elements to prune non-visible breakpoints from the tree. So instead of getting hidden with CSS they'd actually be destroyed this time.

Would that be enough?

[gaearon]

I see there's this nuance.

This prevents life-cycle methods from firing in hidden components and unused html being re-written to the DOM.

I'm not sure this is possible to avoid during hydration. Unless you do something tricky with Suspense maybe. I'll ask the team.

[gaearon]

Let's keep this open for a bit so we can decide on the recommendation. (I can't promise the recommendation won't be that this simply isn't supported, since it wasn't an officially supported way before either, but I'd like to have some concrete conclusion for this use case.)

[KrustyC]

@gaearon if it can help I created a small Codesandbox which replicates my issue. The repo is set up with NetxJs, and styled-components as this is what we have in our app, I hope it can help.

https://codesandbox.io/s/gallant-hopper-kyb05e

[gurkerl83]

The relevant parts in fresnel are its provider, as any provider high in the tree.
https://github.com/artsy/fresnel/blob/70cba29140769d3c147d5865d3bbd661b763e86d/src/Media.tsx#L351

There is an options "disableDynamicMediaQueries" with two branches.

1. When set "true", React 18 works but the result is that life-cycle methods are not prevented from firing in hidden components.

https://github.com/artsy/fresnel/blob/70cba29140769d3c147d5865d3bbd661b763e86d/src/Media.tsx#L354

This does not prevent life-cycle methods from firing in hidden components and unused html being re-written to the DOM.

2. When set to "false" (default), React 18 breaks. This is relevant for the real interesting use case.

https://github.com/artsy/fresnel/blob/70cba29140769d3c147d5865d3bbd661b763e86d/src/Media.tsx#L362

This does prevent life-cycle methods from firing in hidden components and unused html being re-written to the DOM.

And there are is the wrapper component you put around a real component, it is consuming the context.

https://github.com/artsy/fresnel/blob/70cba29140769d3c147d5865d3bbd661b763e86d/src/Media.tsx#L428

What was discussed in this topic.

Discovered the exact commit in React, which lead to the problem in the beginning and why the title of this issue contains "throwing errors" at least in the past. 3f5ff16

I have also provided two sandboxes which illustrates the problem.

Note: The sandboxes do not use the final version of React 18, but they reproduce the most significant part of the problem!

Working version (from a secondary branch in the repo, with Reacts last working experimental version 18.0.0-rc.0-next-fa816be7f-20220128 - about a month old - relative in time when the comment was created)

https://codesandbox.io/s/dawn-meadow-77t84i

Broken version (from repos main branch, with Reacts latest experimental version, a few hours old - relative in time when the comment was created now with final version of React 18)

https://codesandbox.io/s/sad-bell-16ucyf

I will also continue some research on this topic, anyway; thx for coming back to this topic.

[KrustyC]

@gurkerl83 In the sandbox I provided above, if I set disableDynamicMediaQueries to true in the provider, the hydration error does not fire anymore, as you suggest, but it still generates a warning "Prop 'className' did not match." which I think is another potential issue on top of firing not being prevented.

[gaearon]

When set "true", React 18 works but the result is that life-cycle methods are not prevented from firing in hidden components.

I think this is probably the most practical approach. What is the downside of firing lifecycle methods, aside from this being unnecessary / somewhat hurting perf?

[gurkerl83]

@gaearon see here for lifecycle fireings… #23381 (comment)

When Se to true the DOM elements of the children of the component which should not render are pushed to the DOM because an actual render happens, this is also why the lifecycles are executed.

[gaearon]

I understand why they’re executed, I’m asking more about why that’s a problem.

[gurkerl83]

@gaearon Please see the referenced comment, it should provide a description to the problem, e.g. the mentioned double API calls in hooks like effect etc…

[alloy]

@gaearon The problem is that it might trigger unexpected side-effects. In our case, at the time when we designed Fresnel, an example was analytics being sent about what component the user was looking at—we wouldn't want to report all variants, but only the one actually visible.

This might have been solvable in different ways, I don’t recall which ones we evaluated, but we wanted this to be transparent to components especially for cases where a component tree might be using 3rd party code that internally relied on life-cycle events.

[gaearon]

The supported solutions are:

• Render all variants and let all effects fire.
• Or patch up the HTML tree to prune the invisible branches before starting hydration.

I understand the frustration here, but the previous solution printed errors in the console. This means it was not supported. We always considered mismatches bugs in the application code that need to be fixed.

There may be some sort of "smart" workarounds possible with 18. For example, wrapping trees in <Suspense> (which defers their hydration) and making sure you update state to delete the extra variants before they have a chance to hydrate. Then effects from those branches shouldn't fire. But this is probably not something a library should do since <Suspense> boundaries should be specified by the user. I'm sorry I don't have a better solution for your use case.

In the future, we'd like to add a feature to React that would let you render multiple variants on the server, and have the streaming renderer runtime "pick" the right one on the client. Whether based on media query, localStorage, etc. But this is not something that exists today.

[alloy]

@gaearon Thanks for the follow-up 👍 Since originally working on Fresnel I’ve been a little out of the loop of using latest React; could you link to/elaborate on why library code shouldn’t introduce Suspense boundaries? Is it because you can’t be specific about what throw to suspend for (ie catch)?

[gaearon]

Suspense boundaries let the user specify a fallback UI. A library can’t know what fallback to specify since it’s part of the user’s visual design. A library also can’t make assumptions about the correct granularity of Suspense boundaries. They’re really a visual design concept and need to be intentionally designed. So they belong in the application code.

[alloy]

Gotcha, makes sense. Thanks

[gurkerl83]

@gaearon @salazarm I need your assistance to figure out what is wrong with the interpretation I have.

I understand that when a ticket is closed the problem seems to be fixed or followed up in another ticket.

To solve the problem after all I follow Dan's second suggestion.

...
Or patch up the HTML tree to prune the invisible branches before starting hydration.

It seems it is impossible to prune on the client side before starting hydration. The only way possible is involving an effect and a state. Even with a suspense boundary in place it is not possible.

Observation
Removing the throw statement in tryToClaimNextHydratableInstance works! An effect and state mutation is no longer required when handled inside a Suspense boundary.

The throw statement in popHydrationState ensures that an error is thrown if there are still actual differences between the server and client that have not been previously resolved through client intervention in a Suspense boundary.

I ask you to revisit the comments about the repetitive throw events in the samples.

The function tryToClaimNextHydratableInstance gets invoked inside the update functions of

• HostComponent - updateHostComponent on mount
• SuspenseComponent - updateSuspenseComponent on mount
• HostText - updateHostText on mount but not so interesting

The base question is if the throw statements in the function tryToClaimNextHydratableInstance are too early and unnecessary; the second sample extracted from popHydrationState throws them again and mentions an interesting comment.

Note:
Before the function tryToClaimNextHydratableInstance did not have any logic regarding the older DOM-Patching or the new approach followed to throw an error and catch them calling clear container.

Question???
Does an incomplete component (Suspense) don't need the time and opportunity to evolve and resolve the embodied promises (unknown what the shape of the return component is) before it gets canceled? If so this does not seem to be recognised because invoking tryToClaimNextHydratableInstance seems immediate in the update functions from above (on the mount).

function tryToClaimNextHydratableInstance(fiber: Fiber): void {
  if (!isHydrating) {
    return;
  }
  let nextInstance = nextHydratableInstance;
  if (!nextInstance) {
    if (shouldClientRenderOnMismatch(fiber)) {
      warnNonhydratedInstance((hydrationParentFiber: any), fiber);
      throwOnHydrationMismatch(fiber);            <== !!!!!!!!! This is too strict; client-side 
                                                  render decisions are not respected
                                                  before hydration starts. A typical 
                                                  use case is to avoid the initial render 
                                                  when the client has more info such as
                                                  screen size; currently passing an effect 
                                                  executing a state mutation is required; 
                                                  client-side render decisions need an 
                                                  additional render on the client. 

                                                  Unfortunately this is required even when
                                                  render-decisions and components are
                                                  embedded inside a Suspense boundary. :'( 
                                                  !!!!!!!!!
    }
    // Nothing to hydrate. Make it an insertion.
    insertNonHydratedInstance((hydrationParentFiber: any), fiber);
    isHydrating = false;
    hydrationParentFiber = fiber;
    return;
  }
  const firstAttemptedInstance = nextInstance;
  if (!tryHydrate(fiber, nextInstance)) {
    if (shouldClientRenderOnMismatch(fiber)) {
      warnNonhydratedInstance((hydrationParentFiber: any), fiber);
      throwOnHydrationMismatch(fiber);       <== !!!!!!!!! Also too strict; 
                                             same as previous comment 
                                             !!!!!!!!!
    }
    // If we can't hydrate this instance let's try the next one.
    // We use this as a heuristic. It's based on intuition and not data so it
    // might be flawed or unnecessary.
    nextInstance = getNextHydratableSibling(firstAttemptedInstance);
    const prevHydrationParentFiber: Fiber = (hydrationParentFiber: any);
    if (!nextInstance || !tryHydrate(fiber, nextInstance)) {
      // Nothing to hydrate. Make it an insertion.
      insertNonHydratedInstance((hydrationParentFiber: any), fiber);
      isHydrating = false;
      hydrationParentFiber = fiber;
      return;
    }
    // We matched the next one, we'll now assume that the first one was
    // superfluous and we'll delete it. Since we can't eagerly delete it
    // we'll have to schedule a deletion. To do that, this node needs a dummy
    // fiber associated with it.
    deleteHydratableInstance(prevHydrationParentFiber, firstAttemptedInstance);
  }
}

The function popHydrationState gets called after tryToClaimNextHydratableInstance in the respective update functions from above.

I copied the relevant section of the popHydrationState function. The comment in the code makes me wonder if this is the only position where the throw of a hydration mismatch makes sense. As mentioned above, the popHydrationState gets executed after the update functions for Host, Suspense, and Text.

function popHydrationState(fiber: Fiber): boolean {
  ...
  // If we have any remaining hydratable nodes, we need to delete them now.
  // We only do this deeper than head and body since they tend to have random
  // other nodes in them. We also ignore components with pure text content in
  // side of them. We also don't delete anything inside the root container.
  if (
    fiber.tag !== HostRoot &&
    (fiber.tag !== HostComponent ||
      (shouldDeleteUnhydratedTailInstances(fiber.type) &&
        !shouldSetTextContent(fiber.type, fiber.memoizedProps)))
  ) {
    let nextInstance = nextHydratableInstance;
    if (nextInstance) {
      if (shouldClientRenderOnMismatch(fiber)) {
        warnIfUnhydratedTailNodes(fiber);
        throwOnHydrationMismatch(fiber);        <== !!!!!!!!! Reasonable, not to patch the DOM, 
                                                but this is handled by the application directly;
                                                see comments from the previous sample 
                                                !!!!!!!!!
      } else {
        while (nextInstance) {
          deleteHydratableInstance(fiber, nextInstance);
          nextInstance = getNextHydratableSibling(nextInstance);
        }
      }
    }
  }
...
}

Thx!

[arackaf]

@gaearon sorry to dig up an old thread. Apologies if this is not the best place to ask.

It seems like mismatches in a shadow dom are not reported as hydration warnings / errors. Is that by design?

If I render a web-component from the server, which then attaches a shadow dom, and adds content before hydration, I get no warnings / errors and everything works fine.

Would I be reasonably safe in assuming that will not break in the future?

/cc @sebmarkbage

[gaearon]

Can you make a small demo of what you mean?

[arackaf]

@gaearon oof - not easily. Don't know of any repl's that are Next-aware.

It boils down to, if I have a web component, and its registration script is server rendered, so it's "active" the moment the server renders the tags, and inside connectedCallback I do

this.appendChild(someDivElement)

I'll get a hydration warning from React, since Next SSR'd

but the hydrated React saw

BUT if I attached a shadowDom, instead, and start dumping content in it then React doesn't seem to notice, or care.

Would I be correct in assuming that React's hydration mismatch check only looks at light dom content, and doesn't check shadow dom content?

[gaearon]

Would I be correct in assuming that React's hydration mismatch check only looks at light dom content, and doesn't check shadow dom content?

That sounds correct to me. cc @josepharhar

Just in case, you might want to use the @experimental versions which had a bunch of changes to how we handle custom elements. But we definitely don't recurse on the shadow DOM.

[arackaf]

Ah thanks @gaearon. I'm actually extremely excited for the experimental changes - can't wait for web component prop interop :D

But we definitely don't recurse on the shadow DOM

That seems like the slam dunk I was looking for.

Thanks again!

[josepharhar]

If I render a web-component from the server, which then attaches a shadow dom, and adds content before hydration, I get no warnings / errors and everything works fine.

Would I be reasonably safe in assuming that will not break in the future?

In the distant future if there exists a standardized way to server render custom elements and React adds support for this and other browsers add support for declarative shadow dom, then React could emit shadow dom for these custom elements when server rendering.

However, the hydration process for that would be entirely up to the custom element itself, and I don't think it would be appropriate for React to be looking into that shadow dom for any reason.

So yeah, you are safe in assuming that React won't be looking in shadow dom during hydration.

[arackaf]

@josepharhar thank you SO much!