From 235a6c4af67e3e1fbfab7088c857265e0c95b81f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 6 Mar 2020 11:10:01 -0800 Subject: [PATCH] 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. --- packages/react-noop-renderer/src/ReactNoop.js | 1 + .../src/createReactNoop.js | 26 ++++++++++ .../react-reconciler/src/ReactFiberThrow.js | 2 + ...tSuspenseWithNoopRenderer-test.internal.js | 49 +++++++++++++++++++ 4 files changed, 78 insertions(+) diff --git a/packages/react-noop-renderer/src/ReactNoop.js b/packages/react-noop-renderer/src/ReactNoop.js index eb9c8468bcb34..e463b77aa9b1a 100644 --- 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 index 9987343b90f3b..7233f7976409a 100644 --- 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 index 94ef5ba8225b6..df2154db9d5cd 100644 --- 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 index 0df7431977cf2..530544d6cada6 100644 --- 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 ( + + + + ); + } + + 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(); + }); + 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 () => {