Deletion effects should fire parent -> child (#20584)
* Test: Deletion effects should fire parent -> child Regression in new effect implementation * Fix passive deletion effect ordering
Andrew Clark committed
Jan 13, 2021 at 18:18 UTC
a656ace8da6d005e49a750da3d9a3fd0fdcac76c
3 files changed
+100
-10
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+7
-5
@@ -2005,6 +2005,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
2005
) {
2006
while (nextEffect !== null) {
2007
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
+
2015
const child = fiber.child;
2016
// TODO: Only traverse subtree if it has a PassiveStatic flag
2017
if (child !== null) {
@@ -2023,11 +2030,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
2030
) {
2031
while (nextEffect !== null) {
2032
const fiber = nextEffect;
2026
- // TODO: Check if fiber has a PassiveStatic flag
2027
- setCurrentDebugFiberInDEV(fiber);
2028
- commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
2029
- resetCurrentDebugFiberInDEV();
2030
-
2033
if (fiber === deletedSubtreeRoot) {
2034
nextEffect = null;
2035
return;
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+7
-5
@@ -2005,6 +2005,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
2005
) {
2006
while (nextEffect !== null) {
2007
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
+
2015
const child = fiber.child;
2016
// TODO: Only traverse subtree if it has a PassiveStatic flag
2017
if (child !== null) {
@@ -2023,11 +2030,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
2030
) {
2031
while (nextEffect !== null) {
2032
const fiber = nextEffect;
2026
- // TODO: Check if fiber has a PassiveStatic flag
2027
- setCurrentDebugFiberInDEV(fiber);
2028
- commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
2029
- resetCurrentDebugFiberInDEV();
2030
-
2033
if (fiber === deletedSubtreeRoot) {
2034
nextEffect = null;
2035
return;
packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js
new
+86
@@ -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
+});