Bugfix: Missing mode check in resetChildLanes (#18961)

Deferring a hidden tree is only supported in Concurrent Mode.

The missing check leads to an infinite loop when an update is scheduled
inside a hidden tree, because the pending work never gets reset.

This "accidentally" worked in the old reconciler because the heurstic
we used to detect offscreen trees was if `childExpirationTime`
was `Never`.

In the new reconciler, we check the tag instead. Which means we also
need to check the mode, like we do in the begin phase.

We should move this check out of the hot path. It shouldn't have been
in the hot path of the old reconciler, either.

Probably by moving `resetChildLanes` into the switch statement
in ReactFiberCompleteWork.
This commit is contained in:
Andrew Clark 2020-05-19 23:10:49 -07:00 committed by GitHub
parent 95ea8ed47c
commit 8b3b5c3524
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 90 additions and 1 deletions

View File

@ -1684,7 +1684,8 @@ function resetChildLanes(completedWork: Fiber) {
(completedWork.tag === LegacyHiddenComponent || (completedWork.tag === LegacyHiddenComponent ||
completedWork.tag === OffscreenComponent) && completedWork.tag === OffscreenComponent) &&
completedWork.memoizedState !== null && completedWork.memoizedState !== null &&
!includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) !includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) &&
(completedWork.mode & ConcurrentMode) !== NoLanes
) { ) {
// The children of this component are hidden. Don't bubble their // The children of this component are hidden. Don't bubble their
// expiration times. // expiration times.

View File

@ -2,6 +2,7 @@ let React;
let ReactNoop; let ReactNoop;
let Scheduler; let Scheduler;
let LegacyHidden; let LegacyHidden;
let useState;
describe('ReactOffscreen', () => { describe('ReactOffscreen', () => {
beforeEach(() => { beforeEach(() => {
@ -11,6 +12,7 @@ describe('ReactOffscreen', () => {
ReactNoop = require('react-noop-renderer'); ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler'); Scheduler = require('scheduler');
LegacyHidden = React.unstable_LegacyHidden; LegacyHidden = React.unstable_LegacyHidden;
useState = React.useState;
}); });
function Text(props) { function Text(props) {
@ -77,4 +79,90 @@ describe('ReactOffscreen', () => {
</>, </>,
); );
}); });
// @gate experimental
// @gate new
it('does not defer in legacy mode', async () => {
let setState;
function Foo() {
const [state, _setState] = useState('A');
setState = _setState;
return <Text text={state} />;
}
const root = ReactNoop.createLegacyRoot();
await ReactNoop.act(async () => {
root.render(
<>
<LegacyHidden mode="hidden">
<Foo />
</LegacyHidden>
<Text text="Outside" />
</>,
);
// Should not defer the hidden tree
expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
});
expect(root).toMatchRenderedOutput(
<>
<span prop="A" />
<span prop="Outside" />
</>,
);
// Test that the children can be updated
await ReactNoop.act(async () => {
setState('B');
});
expect(Scheduler).toHaveYielded(['B']);
expect(root).toMatchRenderedOutput(
<>
<span prop="B" />
<span prop="Outside" />
</>,
);
});
// @gate experimental
// @gate new
it('does not defer in blocking mode', async () => {
let setState;
function Foo() {
const [state, _setState] = useState('A');
setState = _setState;
return <Text text={state} />;
}
const root = ReactNoop.createBlockingRoot();
await ReactNoop.act(async () => {
root.render(
<>
<LegacyHidden mode="hidden">
<Foo />
</LegacyHidden>
<Text text="Outside" />
</>,
);
// Should not defer the hidden tree
expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
});
expect(root).toMatchRenderedOutput(
<>
<span prop="A" />
<span prop="Outside" />
</>,
);
// Test that the children can be updated
await ReactNoop.act(async () => {
setState('B');
});
expect(Scheduler).toHaveYielded(['B']);
expect(root).toMatchRenderedOutput(
<>
<span prop="B" />
<span prop="Outside" />
</>,
);
});
}); });