8000 Deletion effects should fire parent -> child (#20584) · facebook/react@a656ace · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a656ace

Browse files
acdliteacdlite
authored
Deletion effects should fire parent -> child (#20584)
* Test: Deletion effects should fire parent -> child Regression in new effect implementation * Fix passive deletion effect ordering
1 parent fc07b07 commit a656ace

File tree

3 files changed

+100
-10
lines changed

3 files changed

+100
-10
lines changed

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2005,6 +2005,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
20052005
) {
20062006
while (nextEffect !== null) {
20072007
const fiber = nextEffect;
2008+
2009+
// Deletion effects fire in parent -> child order
2010+
// TODO: Check if fiber has a PassiveStatic flag
2011+
setCurrentDebugFiberInDEV(fiber);
2012+
commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
2013+
resetCurrentDebugFiberInDEV();
2014+
20082015
const child = fiber.child;
20092016
// TODO: Only traverse subtree if it has a PassiveStatic flag
20102017
if (child !== null) {
@@ -2023,11 +2030,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
20232030
) {
20242031
while (nextEffect !== null) {
20252032
const fiber = nextEffect;
2026-
// TODO: Check if fiber has a PassiveStatic flag
2027-
setCurrentDebugFiberInDEV(fiber);
2028-
commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
2029-
resetCurrentDebugFiberInDEV();
2030-
20312033
if (fiber === deletedSubtreeRoot) {
20322034
nextEffect = null;
20332035
return;

‎packages/react-reconciler/src/ReactFiberCommitWork.old.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2005,6 +2005,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
20052005
) {
20062006
while (nextEffect !== null) {
20072007
const fiber = nextEffect;
2008+
2009+
// Deletion effects fire in parent -> child order
2010+
// TODO: Check if fiber has a PassiveStatic flag
2011+
setCurrentDebugFiberInDEV(fiber);
2012+
commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
2013+
resetCurrentDebugFiberInDEV();
2014+
20082015
const child = fiber.child;
20092016
// TODO: Only traverse subtree if it has a PassiveStatic flag
20102017
if (child !== null) {
@@ -2023,11 +2030,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
20232030
) {
20242031
while (nextEffect !== null) {
20252032
const fiber = nextEffect;
2026-
// TODO: Check if fiber has a PassiveStatic flag
2027-
setCurrentDebugFiberInDEV(fiber);
2028-
commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
2029-
resetCurrentDebugFiberInDEV();
2030-
20312033
if (fiber === deletedSubtreeRoot) {
20322034
nextEffect = null;
20332035
return;

‎packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails react-core
8+
* @jest-environment node
9+
*/
10+
11+
/* eslint-disable no-func-assign */
12+
13+
'use strict';
14+
15+
let React;
16+
let ReactNoop;
17+
let Scheduler;
18+
let useEffect;
19+
let useLayoutEffect;
20+
21+
describe('ReactHooksWithNoopRenderer', () => {
22+
beforeEach(() => {
23+
jest.resetModules();
24+
jest.useFakeTimers();
25+
26+
React = require('react');
27+
ReactNoop = require('react-noop-renderer');
28+
Scheduler = require('scheduler');
29+
useEffect = React.useEffect;
30+
useLayoutEffect = React.useLayoutEffect;
31+
});
32+
33+
test('layout unmmouts on deletion are fired in parent -> child order', async () => {
34+
const root = ReactNoop.createRoot();
35+
36+
function Parent() {
37+
useLayoutEffect(() => {
38+
return () => Scheduler.unstable_yieldValue('Unmount parent');
39+
});
40+
return <Child />;
41+
}
42+
43+
function Child() {
44+
useLayoutEffect(() => {
45+
return () => Scheduler.unstable_yieldValue('Unmount child');
46+
});
47+
return 'Child';
48+
}
49+
50+
await ReactNoop.act(async () => {
51+
root.render(<Parent />);
52+
});
53+
expect(root).toMatchRenderedOutput('Child');
54+
await ReactNoop.act(async () => {
55+
root.render(null);
56+
});
57+
expect(Scheduler).toHaveYielded(['Unmount parent', 'Unmount child']);
58+
});
59+
60+
test('passive unmmouts on deletion are fired in parent -> child order', async () => {
61+
const root = ReactNoop.createRoot();
62+
63+
function Parent() {
64+
useEffect(() => {
65+
return () => Scheduler.unstable_yieldValue('Unmount parent');
66+
});
67+
return <Child />;
68+
}
69+
70+
function Child() {
71+
useEffect(() => {
72+
return () => Scheduler.unstable_yieldValue('Unmount child');
73+
});
74+
return 'Child';
75+
}
76+
77+
await ReactNoop.act(async () => {
78+
root.render(<Parent />);
79+
});
80+
expect(root).toMatchRenderedOutput('Child');
81+
await ReactNoop.act(async () => {
82+
root.render(null);
83+
});
84+
expect(Scheduler).toHaveYielded(['Unmount parent', 'Unmount child']);
85+
});
86+
});

0 commit comments

Comments
 (0)
0