Suspending inside a constructor outside of strict mode (#13200)

* Suspending inside a constructor outside of strict mode

Outside of strict mode, suspended components commit in an incomplete
state, then are synchronously deleted in a subsequent commit. If a
component suspends inside the constructor, it mounts without
an instance.

This breaks at least one invariant: during deletion, we assume that
every mounted component has an instance, and check the instance for
the existence of `componentWillUnmount`.

Rather than add a redundant check to the deletion of every class
component, components that suspend inside their constructor and outside
of strict mode are turned into empty functional components before they
are mounted. This is a bit weird, but it's an edge case, and the empty
component will be synchronously unmounted regardless.

* Do not fire lifecycles of a suspended component

In non-strict mode, suspended components commit, but their lifecycles
should not fire.

Author: acdlite

packages/react-reconciler/src/ReactFiberUnwindWork.js

@@ -30,6 +30,7 @@ import {
   NoEffect,
   ShouldCapture,
   Update as UpdateEffect,
+  LifecycleEffectMask,
 } from 'shared/ReactTypeOfSideEffect';
 import {
   enableGetDerivedStateFromCatch,
@@ -71,6 +72,10 @@ import {
 import {findEarliestOutstandingPriorityLevel} from './ReactFiberPendingPriority';
 import {reconcileChildrenAtExpirationTime} from './ReactFiberBeginWork';
 
+function NoopComponent() {
+  return null;
+}
+
 function createRootErrorUpdate(
   fiber: Fiber,
   errorInfo: CapturedValue<mixed>,
@@ -246,6 +251,22 @@ function throwException(
               sourceFiber.tag = FunctionalComponent;
             }
 
+            if (sourceFiber.tag === ClassComponent) {
+              // We're going to commit this fiber even though it didn't
+              // complete. But we shouldn't call any lifecycle methods or
+              // callbacks. Remove all lifecycle effect tags.
+              sourceFiber.effectTag &= ~LifecycleEffectMask;
+              if (sourceFiber.alternate === null) {
+                // We're about to mount a class component that doesn't have an
+                // instance. Turn this into a dummy functional component instead,
+                // to prevent type errors. This is a bit weird but it's an edge
+                // case and we're about to synchronously delete this
+                // component, anyway.
+                sourceFiber.tag = FunctionalComponent;
+                sourceFiber.type = NoopComponent;
+              }
+            }
+
             // Exit without suspending.
             return;
           }

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

@@ -1282,6 +1282,122 @@ describe('ReactSuspense', () => {
         span('C'),
       ]);
     });
+
+    it('suspends inside constructor', async () => {
+      class AsyncTextInConstructor extends React.Component {
+        constructor(props) {
+          super(props);
+          const text = props.text;
+          try {
+            TextResource.read(cache, [props.text, props.ms]);
+            this.state = {text};
+          } catch (promise) {
+            if (typeof promise.then === 'function') {
+              ReactNoop.yield(`Suspend! [${text}]`);
+            } else {
+              ReactNoop.yield(`Error! [${text}]`);
+            }
+            throw promise;
+          }
+        }
+        render() {
+          ReactNoop.yield(this.state.text);
+          return <span prop={this.state.text} />;
+        }
+      }
+
+      ReactNoop.renderLegacySyncRoot(
+        <Placeholder fallback={<Text text="Loading..." />}>
+          <AsyncTextInConstructor ms={100} text="Hi" />
+        </Placeholder>,
+      );
+
+      expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
+    });
+  });
+
+  it('does not call lifecycles of a suspended component', async () => {
+    class TextWithLifecycle extends React.Component {
+      componentDidMount() {
+        ReactNoop.yield(`Mount [${this.props.text}]`);
+      }
+      componentDidUpdate() {
+        ReactNoop.yield(`Update [${this.props.text}]`);
+      }
+      componentWillUnmount() {
+        ReactNoop.yield(`Unmount [${this.props.text}]`);
+      }
+      render() {
+        return <Text {...this.props} />;
+      }
+    }
+
+    class AsyncTextWithLifecycle extends React.Component {
+      componentDidMount() {
+        ReactNoop.yield(`Mount [${this.props.text}]`);
+      }
+      componentDidUpdate() {
+        ReactNoop.yield(`Update [${this.props.text}]`);
+      }
+      componentWillUnmount() {
+        ReactNoop.yield(`Unmount [${this.props.text}]`);
+      }
+      render() {
+        const text = this.props.text;
+        const ms = this.props.ms;
+        try {
+          TextResource.read(cache, [text, ms]);
+          ReactNoop.yield(text);
+          return <span prop={text} />;
+        } catch (promise) {
+          if (typeof promise.then === 'function') {
+            ReactNoop.yield(`Suspend! [${text}]`);
+          } else {
+            ReactNoop.yield(`Error! [${text}]`);
+          }
+          throw promise;
+        }
+      }
+    }
+
+    function App() {
+      return (
+        <Placeholder
+          delayMs={1000}
+          fallback={<TextWithLifecycle text="Loading..." />}>
+          <TextWithLifecycle text="A" />
+          <AsyncTextWithLifecycle ms={100} text="B" />
+          <TextWithLifecycle text="C" />
+        </Placeholder>
+      );
+    }
+
+    ReactNoop.renderLegacySyncRoot(<App />, () =>
+      ReactNoop.yield('Commit root'),
+    );
+    expect(ReactNoop.clearYields()).toEqual([
+      'A',
+      'Suspend! [B]',
+      'C',
+
+      'Mount [A]',
+      // B's lifecycle should not fire because it suspended
+      // 'Mount [B]',
+      'Mount [C]',
+      'Commit root',
+
+      // In a subsequent commit, render a placeholder
+      'Loading...',
+
+      // A, B, and C are unmounted, but we skip calling B's componentWillUnmount
+      'Unmount [A]',
+      'Unmount [C]',
+
+      // Force delete all the existing children when switching to the
+      // placeholder. This should be a mount, not an update.
+      'Mount [Loading...]',
+    ]);
+    expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
   });
 });
 

packages/shared/ReactTypeOfSideEffect.js

@@ -24,6 +24,9 @@ export const DidCapture = /*            */ 0b00001000000;
 export const Ref = /*                   */ 0b00010000000;
 export const Snapshot = /*              */ 0b00100000000;
 
+// Update & Callback & Ref & Snapshot
+export const LifecycleEffectMask = /*   */ 0b00110100100;
+
 // Union of all host effects
 export const HostEffectMask = /*        */ 0b00111111111;