Skip to content

Commit 0f83a64

Browse files
authored
Regression test: Missing unmount after re-order (#20285)
Adds a regression test for a bug I found in the effects refactor. The bug was that reordering a child that contains passive effects would cause the child to "forget" that it contains passive effects. This is because when a Placement effect is scheduled by the reconciler, it would override all of the fiber's flags, including its "static" ones: ``` child.flags = Placement; ``` The problem is that we use a static flag to use a "static" flag to track that a fiber contains passive effects. So what happens is that when the tree is deleted, the unmount effect is never fired. In the new implementation, the fix is to add the Placement flag without overriding the rest of the bitmask: ``` child.flags |= Placement; ``` (The old implementation doesn't need to be changed because it does not use static flags for this purpose.)
1 parent ebf1589 commit 0f83a64

File tree

1 file changed

+46
-0
lines changed

1 file changed

+46
-0
lines changed

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3370,4 +3370,50 @@ describe('ReactHooksWithNoopRenderer', () => {
33703370
});
33713371
expect(Scheduler).toHaveYielded(['Unmount layout B', 'Unmount passive B']);
33723372
});
3373+
3374+
it('regression: deleting a tree and unmounting its effects after a reorder', async () => {
3375+
const root = ReactNoop.createRoot();
3376+
3377+
function Child({label}) {
3378+
useEffect(() => {
3379+
Scheduler.unstable_yieldValue('Mount ' + label);
3380+
return () => {
3381+
Scheduler.unstable_yieldValue('Unmount ' + label);
3382+
};
3383+
}, [label]);
3384+
return label;
3385+
}
3386+
3387+
await act(async () => {
3388+
root.render(
3389+
<>
3390+
<Child key="A" label="A" />
3391+
<Child key="B" label="B" />
3392+
</>,
3393+
);
3394+
});
3395+
expect(Scheduler).toHaveYielded(['Mount A', 'Mount B']);
3396+
3397+
await act(async () => {
3398+
root.render(
3399+
<>
3400+
<Child key="B" label="B" />
3401+
<Child key="A" label="A" />
3402+
</>,
3403+
);
3404+
});
3405+
expect(Scheduler).toHaveYielded([]);
3406+
3407+
await act(async () => {
3408+
root.render(null);
3409+
});
3410+
3411+
expect(Scheduler).toHaveYielded([
3412+
'Unmount B',
3413+
// In the regression, the reorder would cause Child A to "forget" that it
3414+
// contains passive effects. Then when we deleted the tree, A's unmount
3415+
// effect would not fire.
3416+
'Unmount A',
3417+
]);
3418+
});
33733419
});

0 commit comments

Comments
 (0)