Skip to content

Commit

Permalink
Bugfix: Dropped effects in Legacy Mode Suspense (facebook#18238)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
acdlite committed Mar 19, 2020
1 parent d28bd29 commit 82cf50a
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
26 changes: 26 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberThrow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down

0 comments on commit 82cf50a

Please sign in to comment.