diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index d3082d5f785b8..a048500e9ddf3 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -2005,6 +2005,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( ) { while (nextEffect !== null) { const fiber = nextEffect; + + // Deletion effects fire in parent -> child order + // TODO: Check if fiber has a PassiveStatic flag + setCurrentDebugFiberInDEV(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + resetCurrentDebugFiberInDEV(); + const child = fiber.child; // TODO: Only traverse subtree if it has a PassiveStatic flag if (child !== null) { @@ -2023,11 +2030,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( ) { while (nextEffect !== null) { const fiber = nextEffect; - // TODO: Check if fiber has a PassiveStatic flag - setCurrentDebugFiberInDEV(fiber); - commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); - resetCurrentDebugFiberInDEV(); - if (fiber === deletedSubtreeRoot) { nextEffect = null; return; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 95e48b1e4ae4f..72f801981d30b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -2005,6 +2005,13 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( ) { while (nextEffect !== null) { const fiber = nextEffect; + + // Deletion effects fire in parent -> child order + // TODO: Check if fiber has a PassiveStatic flag + setCurrentDebugFiberInDEV(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + resetCurrentDebugFiberInDEV(); + const child = fiber.child; // TODO: Only traverse subtree if it has a PassiveStatic flag if (child !== null) { @@ -2023,11 +2030,6 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( ) { while (nextEffect !== null) { const fiber = nextEffect; - // TODO: Check if fiber has a PassiveStatic flag - setCurrentDebugFiberInDEV(fiber); - commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); - resetCurrentDebugFiberInDEV(); - if (fiber === deletedSubtreeRoot) { nextEffect = null; return; diff --git a/packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js b/packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js new file mode 100644 index 0000000000000..6e9ffcfb894f5 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactEffectOrdering-test.js @@ -0,0 +1,86 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +/* eslint-disable no-func-assign */ + +'use strict'; + +let React; +let ReactNoop; +let Scheduler; +let useEffect; +let useLayoutEffect; + +describe('ReactHooksWithNoopRenderer', () => { + beforeEach(() => { + jest.resetModules(); + jest.useFakeTimers(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + useEffect = React.useEffect; + useLayoutEffect = React.useLayoutEffect; + }); + + test('layout unmmouts on deletion are fired in parent -> child order', async () => { + const root = ReactNoop.createRoot(); + + function Parent() { + useLayoutEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount parent'); + }); + return ; + } + + function Child() { + useLayoutEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount child'); + }); + return 'Child'; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(root).toMatchRenderedOutput('Child'); + await ReactNoop.act(async () => { + root.render(null); + }); + expect(Scheduler).toHaveYielded(['Unmount parent', 'Unmount child']); + }); + + test('passive unmmouts on deletion are fired in parent -> child order', async () => { + const root = ReactNoop.createRoot(); + + function Parent() { + useEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount parent'); + }); + return ; + } + + function Child() { + useEffect(() => { + return () => Scheduler.unstable_yieldValue('Unmount child'); + }); + return 'Child'; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(root).toMatchRenderedOutput('Child'); + await ReactNoop.act(async () => { + root.render(null); + }); + expect(Scheduler).toHaveYielded(['Unmount parent', 'Unmount child']); + }); +});