Bugfix: Dropped effects in Legacy Mode Suspense (facebook#18238)

acdlite · acdlite · commit 82cf50ab3a0c · 2020-03-19T10:38:39.000-07:00
* Failing: Dropped effects in Legacy Mode Suspense

* Transfer mounted effects on suspend in legacy mode

In legacy mode, a component that suspends bails out and commit in
its previous state. If the component previously had mounted effects,
we must transfer those to the work-in-progress so they don't
get dropped.
diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -785,6 +785,32 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       };
     },
 
+    createLegacyRoot() {
+      const container = {
+        rootID: '' + idCounter++,
+        pendingChildren: [],
+        children: [],
+      };
+      const fiberRoot = NoopRenderer.createContainer(
+        container,
+        LegacyRoot,
+        false,
+        null,
+      );
+      return {
+        _Scheduler: Scheduler,
+        render(children: ReactNodeList) {
+          NoopRenderer.updateContainer(children, fiberRoot, null, null);
+        },
+        getChildren() {
+          return getChildren(container);
+        },
+        getChildrenAsJSX() {
+          return getChildrenAsJSX(container);
+        },
+      };
+    },
+
     getChildrenAsJSX(rootID: string = DEFAULT_ROOT_ID) {
       const container = rootContainers.get(rootID);
       return getChildrenAsJSX(container);
diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js
@@ -199,9 +199,11 @@ function throwException(
       // to render it.
       let currentSource = sourceFiber.alternate;
       if (currentSource) {
+        sourceFiber.updateQueue = currentSource.updateQueue;
         sourceFiber.memoizedState = currentSource.memoizedState;
         sourceFiber.expirationTime = currentSource.expirationTime;
       } else {
+        sourceFiber.updateQueue = null;
         sourceFiber.memoizedState = null;
       }
     }
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
@@ -1467,6 +1467,55 @@ function loadModules({
             'Caught an error: Error in host config.',
           );
         });
+
+        it('does not drop mounted effects', async () => {
+          let never = {then() {}};
+
+          let setShouldSuspend;
+          function App() {
+            const [shouldSuspend, _setShouldSuspend] = React.useState(0);
+            setShouldSuspend = _setShouldSuspend;
+            return (
+              <Suspense fallback="Loading...">
+                <Child shouldSuspend={shouldSuspend} />
+              </Suspense>
+            );
+          }
+
+          function Child({shouldSuspend}) {
+            if (shouldSuspend) {
+              throw never;
+            }
+
+            React.useEffect(() => {
+              Scheduler.unstable_yieldValue('Mount');
+              return () => {
+                Scheduler.unstable_yieldValue('Unmount');
+              };
+            }, []);
+
+            return 'Child';
+          }
+
+          const root = ReactNoop.createLegacyRoot(null);
+          await ReactNoop.act(async () => {
+            root.render(<App />);
+          });
+          expect(Scheduler).toHaveYielded(['Mount']);
+          expect(root).toMatchRenderedOutput('Child');
+
+          // Suspend the child. This puts it into an inconsistent state.
+          await ReactNoop.act(async () => {
+            setShouldSuspend(true);
+          });
+          expect(root).toMatchRenderedOutput('Loading...');
+
+          // Unmount everying
+          await ReactNoop.act(async () => {
+            root.render(null);
+          });
+          expect(Scheduler).toHaveYielded(['Unmount']);
+        });
       });
 
       it('does not call lifecycles of a suspended component', async () => {

Original file line number	Diff line number	Diff line change
`@@ -199,9 +199,11 @@ function throwException(`
`199`	`199`	`// to render it.`
`200`	`200`	`let currentSource = sourceFiber.alternate;`
`201`	`201`	`if (currentSource) {`
	`202`	`+ sourceFiber.updateQueue = currentSource.updateQueue;`
`202`	`203`	`sourceFiber.memoizedState = currentSource.memoizedState;`
`203`	`204`	`sourceFiber.expirationTime = currentSource.expirationTime;`
`204`	`205`	`} else {`
	`206`	`+ sourceFiber.updateQueue = null;`
`205`	`207`	`sourceFiber.memoizedState = null;`
`206`	`208`	`}`
`207`	`209`	`}`