Investigate hydration with Suspense and undefined fallback

[theKashey]

original issue

facebook/react#16938

test which explains everything

https://github.com/facebook/react/pull/16945/files#diff-ab371863932cd2e8f0ba14ff2eaab380R687

---

Mostly for your information.

The last React version which will work out of the box with the provided examples API is 16.9. After that one has to use <Suspense fallback={undefined}> during the hydration, or it will fail.

In other words this moment might be changed a little - renderFallback should return undefined during hydration

react-loosely-lazy/src/suspense/component.tsx

Lines 116 to 118 in f3f19d0

	<Suspense fallback={this.renderFallback(false)}>
	{this.props.children}
	</Suspense>

Questions yet to answer

• can one render Suspense first time without fallback and add it using useEffect a render tick later?

[albertogasparin]

That is indeed what LazySuspense does internally however to do that we need to detect SSR vs non SSR render.
So if you see the error while SSRing, it is because the library thinks that you are running the code in client side mode, and that renders a real Suspense instead of the custom "markers" (so it is not an error due to react > 16.9).
We have a function called isNodeEnvironment that tries to automatically detect if you are running on node, but I guess it returns false on your SSR env so we need to fix that.

Can you provide more info so we can tweak that function? Or do you have better recommendations on how to do that?
One option I thought about was to use the babel plugin to set that value, but a runtime check should be doable anyway 🤔

[theKashey]

So the problem is on the client side, not on the server.
React, since 16.10 "knows" that you cannot use Suspense during SSR, and if sees it during hydration - ignores content inside unless fallback set to undefined.

Yet again - any <Suspense fallback="something/> during hydration will break markup.

Wrapped content is expected to be recreated - https://github.com/sebmarkbage/react/blob/185700696ebbe737c99bd6c4b678d5f2a923bd29/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js#L668-L682

The problem was discovered not with loosely implementation, but reported for imported-components. After a quick investigation I've found the root cause and here the story ends.
From now on my duty is to fix the problem in the projects I maintain, and let other projects aware of the problem.

[albertogasparin]

Ah, I see what you mean. I actually didn't know about the <Suspense fallback={undefined} /> option. I've found a different solution (render the Suspense boundary only after hydration) but might investigate if, as you say, we can simply getting it working by rendering undefined.
Given with the current trick we are already supporting React 16.10+ (and React 17 beta), I'll mark this as enhancement

[theKashey]

Not sure how you render Suspense boundary after hydration as long as that should lead to local tree remount. And could not find corresponding logic in the code.

[albertogasparin]

Sorry, I wrote that wrongly. What I meant to say is that we don't really use Suspense fallback for hydration, as the content of SSR is collected and rendered by a sibling component. The flow is:

1. LooselyLazy collects all html for render/hydration on init
2. LazySuspense renders children inside Suspense with fallback prop rendered by DynamicFallback (but never happens really)
3. children finds the corresponding hydration html and calls setFallback on LazySuspense
4. children renders React.lazy, which throws an error promise that hits the boundary straight away
5. LazySuspense Suspense boundary re-renders to render the fallback, but this time DynamicFallback output changes as hydrationFallback has been set (as setFallback is sync) and it returns null
6. LazySuspense resumes its content rendering, and renders a Suspense sibling with hydration html (again DynamicFallback does the magic here as is able to consume the fresh hydrationFallback value)

So at this point, inside LazySuspense, what we have is a Suspense boundary that holds the lazy promise, rendering null as a fallback followed by a sibling that is rendering the hydrated fallback.
As soon as Suspense is done with the promises and unmounts DynamicFallback, we clean the hydration html value and make LazySuspense re-render, killing the Suspense sibling from the tree and let React render the content inside Suspense normally.

Didn't have time to fully investigate the performance implications of this last step (as whenever React recreates all DOM nodes or if it picks them up somehow) so that might be where your suggestion of using undefined instead of the current special render-prop component could be performing better.

[theKashey]

From what I know (spent a few years fighting among HotLoader) - React always discards the old tree on component change. Replacing fallback with actual component might resolve to the same HTML, but the original DOM tree would be rejected.
So it should have some perf implications (at least non properly cached images might "blink"), and the question is that implication bearable or not quite.

PS: Your partial hydration implementation without the need of wrapping div is 🥳
PPS: That might be a solution for "non nested" splitting points - if the actual LazySuspense can detect "content is inside", then it might skip own hydration via suppressHydrationWarning, but that would require outer DOM node
https://gist.github.com/theKashey/8b54d9a5c050d8faa929aad9682aa485#file-hydrate-js-L28-L32