Skip to content

Commit f7a3efd

Browse files
committed
fix deleted components added to the memoized updaters list
1 parent 8116347 commit f7a3efd

File tree

3 files changed

+177
-8
lines changed

3 files changed

+177
-8
lines changed

packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import {
8080
LayoutMask,
8181
PassiveMask,
8282
Visibility,
83+
Deletion,
8384
} from './ReactFiberFlags';
8485
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
8586
import {
@@ -1212,6 +1213,8 @@ function commitUnmount(
12121213
): void {
12131214
onCommitUnmount(current);
12141215

1216+
current.flags |= Deletion;
1217+
12151218
switch (current.tag) {
12161219
case FunctionComponent:
12171220
case ForwardRef:

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes';
1212
import type {Lanes, Lane} from './ReactFiberLane.new';
1313
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
1414
import type {StackCursor} from './ReactFiberStack.new';
15-
import type {Flags} from './ReactFiberFlags';
15+
import {ChildDeletion, Deletion, Flags} from './ReactFiberFlags';
1616
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
1717

1818
import {
@@ -486,7 +486,9 @@ export function scheduleUpdateOnFiber(
486486
// example, during an input event.
487487
if (enableUpdaterTracking) {
488488
if (isDevToolsPresent) {
489-
addFiberToLanesMap(root, fiber, lane);
489+
if ((fiber.flags & Deletion) === NoFlags) {
490+
addFiberToLanesMap(root, fiber, lane);
491+
}
490492
}
491493
}
492494

packages/react-reconciler/src/__tests__/ReactUpdaters-test.internal.js

Lines changed: 170 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,14 @@ describe('updaters', () => {
4141
}
4242
const schedulerTags = [];
4343
const schedulerTypes = [];
44-
fiberRoot.memoizedUpdaters.forEach(fiber => {
45-
schedulerTags.push(fiber.tag);
46-
schedulerTypes.push(fiber.elementType);
47-
});
48-
allSchedulerTags.push(schedulerTags);
49-
allSchedulerTypes.push(schedulerTypes);
44+
if (fiberRoot.memoizedUpdaters.size > 0) {
45+
fiberRoot.memoizedUpdaters.forEach(fiber => {
46+
schedulerTags.push(fiber.tag);
47+
schedulerTypes.push(fiber.elementType);
48+
});
49+
allSchedulerTags.push(schedulerTags);
50+
allSchedulerTypes.push(schedulerTypes);
51+
}
5052
}),
5153
onCommitUnmount: jest.fn(() => {}),
5254
onPostCommitRoot: jest.fn(() => {}),
@@ -435,4 +437,166 @@ describe('updaters', () => {
435437
// Verify no outstanding flushes
436438
Scheduler.unstable_flushAll();
437439
});
440+
441+
it('components that were deleted should not be added to updaters during the layout phase', () => {
442+
const {
443+
FunctionComponent,
444+
HostRoot,
445+
} = require('react-reconciler/src/ReactWorkTags');
446+
447+
let setBarUnmounted;
448+
function Bar() {
449+
const [, setState] = React.useState(false);
450+
Scheduler.unstable_yieldValue('Bar render');
451+
452+
React.useLayoutEffect(() => {
453+
return () => {
454+
Scheduler.unstable_yieldValue('useLayoutEffect Bar unmount');
455+
setState(true);
456+
};
457+
});
458+
459+
return null;
460+
}
461+
462+
function App() {
463+
Scheduler.unstable_yieldValue('App render');
464+
const [barUnmounted, _setBarUnmounted] = React.useState(false);
465+
setBarUnmounted = _setBarUnmounted;
466+
return <>{!barUnmounted && <Bar />}</>;
467+
}
468+
const root = ReactDOM.createRoot(document.createElement('div'));
469+
act(() => {
470+
root.render(<App />);
471+
});
472+
473+
expect(Scheduler).toHaveYielded([
474+
'App render',
475+
'Bar render',
476+
'onCommitRoot',
477+
]);
478+
expect(allSchedulerTags).toEqual([[HostRoot]]);
479+
480+
act(() => {
481+
setBarUnmounted(true);
482+
expect(Scheduler).toFlushAndYield([
483+
'App render',
484+
'useLayoutEffect Bar unmount',
485+
'onCommitRoot',
486+
'onCommitRoot',
487+
]);
488+
});
489+
490+
expect(allSchedulerTags).toEqual([[HostRoot], [FunctionComponent]]);
491+
expect(allSchedulerTypes).toEqual([[null], [App]]);
492+
});
493+
494+
it('components in a deleted subtree should not be added to updaters during the layout phase', () => {
495+
const {
496+
FunctionComponent,
497+
HostRoot,
498+
} = require('react-reconciler/src/ReactWorkTags');
499+
500+
function Foo() {
501+
return <Bar />;
502+
}
503+
504+
let setBarUnmounted;
505+
function Bar() {
506+
const [, setState] = React.useState(false);
507+
Scheduler.unstable_yieldValue('Bar render');
508+
509+
React.useLayoutEffect(() => {
510+
return () => {
511+
Scheduler.unstable_yieldValue('useLayoutEffect Bar unmount');
512+
setState(true);
513+
};
514+
});
515+
516+
return null;
517+
}
518+
519+
function App() {
520+
Scheduler.unstable_yieldValue('App render');
521+
const [barUnmounted, _setBarUnmounted] = React.useState(false);
522+
setBarUnmounted = _setBarUnmounted;
523+
return <>{!barUnmounted && <Foo />}</>;
524+
}
525+
const root = ReactDOM.createRoot(document.createElement('div'));
526+
act(() => {
527+
root.render(<App />);
528+
});
529+
530+
expect(Scheduler).toHaveYielded([
531+
'App render',
532+
'Bar render',
533+
'onCommitRoot',
534+
]);
535+
expect(allSchedulerTags).toEqual([[HostRoot]]);
536+
537+
act(() => {
538+
setBarUnmounted(true);
539+
expect(Scheduler).toFlushAndYield([
540+
'App render',
541+
'useLayoutEffect Bar unmount',
542+
'onCommitRoot',
543+
'onCommitRoot',
544+
]);
545+
});
546+
547+
expect(allSchedulerTags).toEqual([[HostRoot], [FunctionComponent]]);
548+
expect(allSchedulerTypes).toEqual([[null], [App]]);
549+
});
550+
551+
it('components that were deleted should not be added to updaters during the passive phase', () => {
552+
const {
553+
FunctionComponent,
554+
HostRoot,
555+
} = require('react-reconciler/src/ReactWorkTags');
556+
557+
let setBarUnmounted;
558+
function Bar() {
559+
const [, setState] = React.useState(false);
560+
Scheduler.unstable_yieldValue('Bar render');
561+
562+
React.useEffect(() => {
563+
return () => {
564+
Scheduler.unstable_yieldValue('usePassiveEffect Bar unmount');
565+
setState(true);
566+
};
567+
});
568+
569+
return null;
570+
}
571+
572+
function App() {
573+
Scheduler.unstable_yieldValue('App render');
574+
const [barUnmounted, _setBarUnmounted] = React.useState(false);
575+
setBarUnmounted = _setBarUnmounted;
576+
return <>{!barUnmounted && <Bar />}</>;
577+
}
578+
const root = ReactDOM.createRoot(document.createElement('div'));
579+
act(() => {
580+
root.render(<App />);
581+
});
582+
583+
expect(Scheduler).toHaveYielded([
584+
'App render',
585+
'Bar render',
586+
'onCommitRoot',
587+
]);
588+
expect(allSchedulerTags).toEqual([[HostRoot]]);
589+
590+
act(() => {
591+
setBarUnmounted(true);
592+
expect(Scheduler).toFlushAndYield([
593+
'App render',
594+
'onCommitRoot',
595+
'usePassiveEffect Bar unmount',
596+
]);
597+
});
598+
599+
expect(allSchedulerTags).toEqual([[HostRoot], [FunctionComponent]]);
600+
expect(allSchedulerTypes).toEqual([[null], [App]]);
601+
});
438602
});

0 commit comments

Comments
 (0)