[Synchronous Suspense] Suspending a class outside concurrent mode (facebook#13926)

* [Synchronous Suspense] Suspending a class outside concurrent mode

When a class component suspends during mount outside concurrent mode,
change the tag so it's not mistaken for a completed component. For
example, we should not call componentWillUnmount if it is deleted.

* PR nits

Author: acdlite

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -33,6 +33,7 @@ import {
   MemoComponent,
   SimpleMemoComponent,
   LazyComponent,
+  IncompleteClassComponent,
 } from 'shared/ReactWorkTags';
 import {
   NoEffect,
@@ -762,7 +763,7 @@ function mountLazyComponent(
   renderExpirationTime,
 ) {
   if (_current !== null) {
-    // An indeterminate component only mounts if it suspended inside a non-
+    // An lazy component only mounts if it suspended inside a non-
     // concurrent tree, in an inconsistent state. We want to tree it like
     // a new mount, even though an empty version of it already committed.
     // Disconnect the alternate pointers.
@@ -840,6 +841,64 @@ function mountLazyComponent(
   return child;
 }
 
+function mountIncompleteClassComponent(
+  _current,
+  workInProgress,
+  Component,
+  nextProps,
+  renderExpirationTime,
+) {
+  if (_current !== null) {
+    // An incomplete component only mounts if it suspended inside a non-
+    // concurrent tree, in an inconsistent state. We want to tree it like
+    // a new mount, even though an empty version of it already committed.
+    // Disconnect the alternate pointers.
+    _current.alternate = null;
+    workInProgress.alternate = null;
+    // Since this is conceptually a new fiber, schedule a Placement effect
+    workInProgress.effectTag |= Placement;
+  }
+
+  // Promote the fiber to a class and try rendering again.
+  workInProgress.tag = ClassComponent;
+
+  // The rest of this function is a fork of `updateClassComponent`
+
+  // Push context providers early to prevent context stack mismatches.
+  // During mounting we don't know the child context yet as the instance doesn't exist.
+  // We will invalidate the child context in finishClassComponent() right after rendering.
+  let hasContext;
+  if (isLegacyContextProvider(Component)) {
+    hasContext = true;
+    pushLegacyContextProvider(workInProgress);
+  } else {
+    hasContext = false;
+  }
+  prepareToReadContext(workInProgress, renderExpirationTime);
+
+  constructClassInstance(
+    workInProgress,
+    Component,
+    nextProps,
+    renderExpirationTime,
+  );
+  mountClassInstance(
+    workInProgress,
+    Component,
+    nextProps,
+    renderExpirationTime,
+  );
+
+  return finishClassComponent(
+    null,
+    workInProgress,
+    Component,
+    true,
+    hasContext,
+    renderExpirationTime,
+  );
+}
+
 function mountIndeterminateComponent(
   _current,
   workInProgress,
@@ -1654,6 +1713,21 @@ function beginWork(
         renderExpirationTime,
       );
     }
+    case IncompleteClassComponent: {
+      const Component = workInProgress.type;
+      const unresolvedProps = workInProgress.pendingProps;
+      const resolvedProps =
+        workInProgress.elementType === Component
+          ? unresolvedProps
+          : resolveDefaultProps(Component, unresolvedProps);
+      return mountIncompleteClassComponent(
+        current,
+        workInProgress,
+        Component,
+        resolvedProps,
+        renderExpirationTime,
+      );
+    }
     default:
       invariant(
         false,

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -32,6 +32,7 @@ import {
   HostPortal,
   Profiler,
   SuspenseComponent,
+  IncompleteClassComponent,
 } from 'shared/ReactWorkTags';
 import {
   invokeGuardedCallback,
@@ -221,6 +222,7 @@ function commitBeforeMutationLifeCycles(
     case HostComponent:
     case HostText:
     case HostPortal:
+    case IncompleteClassComponent:
       // Nothing to do for these component types
       return;
     default: {
@@ -392,6 +394,8 @@ function commitLifeCycles(
       }
       return;
     }
+    case IncompleteClassComponent:
+      break;
     default: {
       invariant(
         false,
@@ -495,7 +499,13 @@ function commitUnmount(current: Fiber): void {
     case ClassComponent: {
       safelyDetachRef(current);
       const instance = current.stateNode;
-      if (typeof instance.componentWillUnmount === 'function') {
+      if (
+        // Typically, a component that mounted will have an instance. However,
+        // outside of concurrent mode, a suspended component may commit without
+        // an instance, so we need to check whether it exists.
+        instance !== null &&
+        typeof instance.componentWillUnmount === 'function'
+      ) {
         safelyCallComponentWillUnmount(current, instance);
       }
       return;
@@ -924,6 +934,9 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
     case SuspenseComponent: {
       return;
     }
+    case IncompleteClassComponent: {
+      return;
+    }
     default: {
       invariant(
         false,

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -37,6 +37,7 @@ import {
   MemoComponent,
   SimpleMemoComponent,
   LazyComponent,
+  IncompleteClassComponent,
 } from 'shared/ReactWorkTags';
 import {Placement, Ref, Update} from 'shared/ReactSideEffectTags';
 import invariant from 'shared/invariant';
@@ -717,6 +718,15 @@ function completeWork(
       break;
     case MemoComponent:
       break;
+    case IncompleteClassComponent: {
+      // Same as class component case. I put it down here so that the tags are
+      // sequential to ensure this switch is compiled to a jump table.
+      const Component = workInProgress.type;
+      if (isLegacyContextProvider(Component)) {
+        popLegacyContext(workInProgress);
+      }
+      break;
+    }
     default:
       invariant(
         false,

packages/react-reconciler/src/ReactFiberUnwindWork.js

@@ -25,6 +25,7 @@ import {
   HostPortal,
   ContextProvider,
   SuspenseComponent,
+  IncompleteClassComponent,
 } from 'shared/ReactWorkTags';
 import {
   DidCapture,
@@ -252,10 +253,12 @@ function throwException(
             // But we shouldn't call any lifecycle methods or callbacks. Remove
             // all lifecycle effect tags.
             sourceFiber.effectTag &= ~LifecycleEffectMask;
-            if (sourceFiber.alternate === null) {
-              // Set the instance back to null. We use this as a heuristic to
-              // detect that the fiber mounted in an inconsistent state.
-              sourceFiber.stateNode = null;
+            const current = sourceFiber.alternate;
+            if (current === null) {
+              // This is a new mount. Change the tag so it's not mistaken for a
+              // completed component. For example, we should not call
+              // componentWillUnmount if it is deleted.
+              sourceFiber.tag = IncompleteClassComponent;
             }
           }
 

packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js

@@ -492,6 +492,45 @@ describe('ReactSuspense', () => {
       expect(root).toMatchRenderedOutput('Stateful: 2B');
     });
 
+    it('suspends in a class that has componentWillUnmount and is then deleted', () => {
+      class AsyncTextWithUnmount extends React.Component {
+        componentWillUnmount() {
+          ReactTestRenderer.unstable_yield('will unmount');
+        }
+        render() {
+          const text = this.props.text;
+          const ms = this.props.ms;
+          try {
+            TextResource.read(cache, [text, ms]);
+            ReactTestRenderer.unstable_yield(text);
+            return text;
+          } catch (promise) {
+            if (typeof promise.then === 'function') {
+              ReactTestRenderer.unstable_yield(`Suspend! [${text}]`);
+            } else {
+              ReactTestRenderer.unstable_yield(`Error! [${text}]`);
+            }
+            throw promise;
+          }
+        }
+      }
+
+      function App({text}) {
+        return (
+          <Suspense fallback={<Text text="Loading..." />}>
+            <AsyncTextWithUnmount text={text} ms={100} />
+          </Suspense>
+        );
+      }
+
+      const root = ReactTestRenderer.create(<App text="A" />);
+      expect(ReactTestRenderer).toHaveYielded(['Suspend! [A]', 'Loading...']);
+      root.update(<Text text="B" />);
+      // Should not fire componentWillUnmount
+      expect(ReactTestRenderer).toHaveYielded(['B']);
+      expect(root).toMatchRenderedOutput('B');
+    });
+
     it('retries when an update is scheduled on a timed out tree', () => {
       let instance;
       class Stateful extends React.Component {

packages/react-test-renderer/src/ReactTestRenderer.js

@@ -29,6 +29,7 @@ import {
   Profiler,
   MemoComponent,
   SimpleMemoComponent,
+  IncompleteClassComponent,
 } from 'shared/ReactWorkTags';
 import invariant from 'shared/invariant';
 import ReactVersion from 'shared/ReactVersion';
@@ -193,6 +194,7 @@ function toTree(node: ?Fiber) {
     case Profiler:
     case ForwardRef:
     case MemoComponent:
+    case IncompleteClassComponent:
       return childrenToTree(node.child);
     default:
       invariant(

packages/shared/ReactWorkTags.js

@@ -45,3 +45,4 @@ export const SuspenseComponent = 13;
 export const MemoComponent = 14;
 export const SimpleMemoComponent = 15;
 export const LazyComponent = 16;
+export const IncompleteClassComponent = 17;