diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index de636e62cff1a..5e35b0404a32e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -1904,6 +1904,14 @@ function resetChildLanes(completedWork: Fiber) { mergeLanes(child.lanes, child.childLanes), ); + // Preserve passive static flag even in the case of a bailout; + // otherwise a subsequent unmount may bailout before calling destroy functions. + subtreeTag |= child.subtreeTag & PassiveStaticSubtreeTag; + const effectTag = child.effectTag; + if ((effectTag & PassiveStatic) !== NoEffect) { + subtreeTag |= PassiveStaticSubtreeTag; + } + treeBaseDuration += child.treeBaseDuration; child = child.sibling; } @@ -1928,9 +1936,19 @@ function resetChildLanes(completedWork: Fiber) { mergeLanes(child.lanes, child.childLanes), ); + // Preserve passive static flag even in the case of a bailout; + // otherwise a subsequent unmount may bailout before calling destroy functions. + subtreeTag |= child.subtreeTag & PassiveStaticSubtreeTag; + const effectTag = child.effectTag; + if ((effectTag & PassiveStatic) !== NoEffect) { + subtreeTag |= PassiveStaticSubtreeTag; + } + child = child.sibling; } } + + completedWork.subtreeTag |= subtreeTag; } completedWork.childLanes = newChildLanes; diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 3296a8ec6dcc0..333e8930e7340 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -2734,6 +2734,144 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([]); }); }); + + it('calls passive effect destroy functions for memoized components', () => { + const Wrapper = ({children}) => children; + function Child() { + React.useEffect(() => { + Scheduler.unstable_yieldValue('passive create'); + return () => { + Scheduler.unstable_yieldValue('passive destroy'); + }; + }, []); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('layout create'); + return () => { + Scheduler.unstable_yieldValue('layout destroy'); + }; + }, []); + Scheduler.unstable_yieldValue('render'); + return null; + } + + const isEqual = (prevProps, nextProps) => + prevProps.prop === nextProps.prop; + const MemoizedChild = React.memo(Child, isEqual); + + act(() => { + ReactNoop.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded([ + 'render', + 'layout create', + 'passive create', + ]); + + // Include at least one no-op (memoized) update to trigger original bug. + act(() => { + ReactNoop.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded([]); + + act(() => { + ReactNoop.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded([ + 'render', + 'layout destroy', + 'layout create', + 'passive destroy', + 'passive create', + ]); + + act(() => { + ReactNoop.render(null); + }); + expect(Scheduler).toHaveYielded(['layout destroy', 'passive destroy']); + }); + + it('calls passive effect destroy functions for descendants of memoized components', () => { + const Wrapper = ({children}) => children; + function Child() { + return ; + } + + function Grandchild() { + React.useEffect(() => { + Scheduler.unstable_yieldValue('passive create'); + return () => { + Scheduler.unstable_yieldValue('passive destroy'); + }; + }, []); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('layout create'); + return () => { + Scheduler.unstable_yieldValue('layout destroy'); + }; + }, []); + Scheduler.unstable_yieldValue('render'); + return null; + } + + const isEqual = (prevProps, nextProps) => + prevProps.prop === nextProps.prop; + const MemoizedChild = React.memo(Child, isEqual); + + act(() => { + ReactNoop.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded([ + 'render', + 'layout create', + 'passive create', + ]); + + // Include at least one no-op (memoized) update to trigger original bug. + act(() => { + ReactNoop.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded([]); + + act(() => { + ReactNoop.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded([ + 'render', + 'layout destroy', + 'layout create', + 'passive destroy', + 'passive create', + ]); + + act(() => { + ReactNoop.render(null); + }); + expect(Scheduler).toHaveYielded(['layout destroy', 'passive destroy']); + }); }); describe('useLayoutEffect', () => {