Client render Suspense content if there's no boundary match (#16945)

Without the enableSuspenseServerRenderer flag there will never be a boundary match. Also when it is enabled, there might not be a boundary match if something was conditionally rendered by mistake.

With this PR it will now client render the content of a Suspense boundary in that case and issue a DEV only hydration warning. This is the only sound semantics for this case.

Unfortunately, landing this will once again break #16938. It will be less bad though because at least it'll just work by client rendering the content instead of hydrating and issue a DEV only warning.

However, we must land this before enabling the enableSuspenseServerRenderer flag since it does this anyway.

I did notice that we special case fallback={undefined} due to our unfortunate semantics for that. So technically a workaround that works is actually setting the fallback to undefined on the server and during hydration. Then flip it on only after hydration. That could be a workaround if you want to be able to have a Suspense boundary work only after hydration for some reason.

It's kind of unfortunate but at least those semantics are internally consistent. So I added a test for that.

Author: sebmarkbage

packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js

@@ -517,14 +517,45 @@ describe('ReactDOMServerHydration', () => {
     });
   }
 
-  it('regression test: Suspense + hydration in legacy mode ', () => {
+  it('Suspense + hydration in legacy mode', () => {
     const element = document.createElement('div');
     element.innerHTML = '<div>Hello World</div>';
+    let div = element.firstChild;
+    let ref = React.createRef();
+    expect(() =>
+      ReactDOM.hydrate(
+        <React.Suspense fallback={null}>
+          <div ref={ref}>Hello World</div>
+        </React.Suspense>,
+        element,
+      ),
+    ).toWarnDev(
+      'Warning: Did not expect server HTML to contain a <div> in <div>.',
+      {withoutStack: true},
+    );
+
+    // The content should've been client rendered and replaced the
+    // existing div.
+    expect(ref.current).not.toBe(div);
+    // The HTML should be the same though.
+    expect(element.innerHTML).toBe('<div>Hello World</div>');
+  });
+
+  it('Suspense + hydration in legacy mode with no fallback', () => {
+    const element = document.createElement('div');
+    element.innerHTML = '<div>Hello World</div>';
+    let div = element.firstChild;
+    let ref = React.createRef();
     ReactDOM.hydrate(
       <React.Suspense>
-        <div>Hello World</div>
+        <div ref={ref}>Hello World</div>
       </React.Suspense>,
       element,
     );
+
+    // Because this didn't have a fallback, it was hydrated as if it's
+    // not a Suspense boundary.
+    expect(ref.current).toBe(div);
+    expect(element.innerHTML).toBe('<div>Hello World</div>');
   });
 });

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -1592,12 +1592,12 @@ function updateSuspenseComponent(
   // children. It's essentially a very basic form of re-parenting.
 
   if (current === null) {
-    if (enableSuspenseServerRenderer) {
-      // If we're currently hydrating, try to hydrate this boundary.
-      // But only if this has a fallback.
-      if (nextProps.fallback !== undefined) {
-        tryToClaimNextHydratableInstance(workInProgress);
-        // This could've been a dehydrated suspense component.
+    // If we're currently hydrating, try to hydrate this boundary.
+    // But only if this has a fallback.
+    if (nextProps.fallback !== undefined) {
+      tryToClaimNextHydratableInstance(workInProgress);
+      // This could've been a dehydrated suspense component.
+      if (enableSuspenseServerRenderer) {
         const suspenseState: null | SuspenseState =
           workInProgress.memoizedState;
         if (suspenseState !== null) {

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -870,10 +870,9 @@ function completeWork(
       const nextDidTimeout = nextState !== null;
       let prevDidTimeout = false;
       if (current === null) {
-        // In cases where we didn't find a suitable hydration boundary we never
-        // put this in dehydrated mode, but we still need to pop the hydration
-        // state since we might be inside the insertion tree.
-        popHydrationState(workInProgress);
+        if (workInProgress.memoizedProps.fallback !== undefined) {
+          popHydrationState(workInProgress);
+        }
       } else {
         const prevState: null | SuspenseState = current.memoizedState;
         prevDidTimeout = prevState !== null;

packages/react-reconciler/src/ReactFiberHydrationContext.js

@@ -404,10 +404,11 @@ function skipPastDehydratedSuspenseInstance(
   let suspenseState: null | SuspenseState = fiber.memoizedState;
   let suspenseInstance: null | SuspenseInstance =
     suspenseState !== null ? suspenseState.dehydrated : null;
-  if (suspenseInstance === null) {
-    // This Suspense boundary was hydrated without a match.
-    return nextHydratableInstance;
-  }
+  invariant(
+    suspenseInstance,
+    'Expected to have a hydrated suspense instance. ' +
+      'This error is likely caused by a bug in React. Please file an issue.',
+  );
   return getNextHydratableInstanceAfterSuspenseInstance(suspenseInstance);
 }