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

* 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.
facebook · Mar 6, 2020 · 235a6c4 · 235a6c4
1 parent 5fbb165
commit 235a6c4
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 0 deletions.
diff --git a/packages/react-noop-renderer/src/ReactNoop.js b/packages/react-noop-renderer/src/ReactNoop.js
@@ -24,6 +24,7 @@ export const {
   getOrCreateRootContainer,
   createRoot,
   createBlockingRoot,
+  createLegacyRoot,
   getChildrenAsJSX,
   getPendingChildrenAsJSX,
   createPortal,

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
@@ -1448,6 +1448,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 () => {