From f961eca4596c7bb9b71993fdde95cf8c810286f6 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Fri, 27 Feb 2026 16:03:38 -0500 Subject: [PATCH 1/2] Fix StrictMode double-invoking effects on keyed child reorder In placeChild, the move case was setting PlacementDEV in addition to Placement. PlacementDEV is meant to identify newly inserted fibers so that StrictMode can double-invoke their effects on mount. Moved fibers are not new insertions, so they should not have PlacementDEV set. This matches the behavior of placeSingleChild, which only sets PlacementDEV when alternate === null (i.e., a true insertion). --- .../react-reconciler/src/ReactChildFiber.js | 2 +- .../src/__tests__/StrictEffectsMode-test.js | 81 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 42f1b70918d3..ad890d350b3a 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -525,7 +525,7 @@ function createChildReconciler( const oldIndex = current.index; if (oldIndex < lastPlacedIndex) { // This is a move. - newFiber.flags |= Placement | PlacementDEV; + newFiber.flags |= Placement; return lastPlacedIndex; } else { // This item can stay in place. diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js index 0d0287bca612..fadc32e2ec8d 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js @@ -808,6 +808,87 @@ describe('StrictEffectsMode', () => { ]); }); + it('should not double-invoke effects when a keyed child is moved', async () => { + const log = []; + function Item({label}) { + React.useEffect(() => { + log.push('useEffect mount ' + label); + return () => log.push('useEffect unmount ' + label); + }, []); + + React.useLayoutEffect(() => { + log.push('useLayoutEffect mount ' + label); + return () => log.push('useLayoutEffect unmount ' + label); + }, []); + + return label; + } + + function App({items}) { + return items.map(item => ); + } + + // Initial mount: [A, B, C] + await act(() => { + ReactNoop.renderToRootWithID( + + + , + 'root', + ); + }); + + // StrictMode double-invokes on mount — this is expected. + if (__DEV__) { + expect(log).toEqual([ + 'useLayoutEffect mount A', + 'useLayoutEffect mount B', + 'useLayoutEffect mount C', + 'useEffect mount A', + 'useEffect mount B', + 'useEffect mount C', + 'useLayoutEffect unmount A', + 'useLayoutEffect unmount B', + 'useLayoutEffect unmount C', + 'useEffect unmount A', + 'useEffect unmount B', + 'useEffect unmount C', + 'useLayoutEffect mount A', + 'useLayoutEffect mount B', + 'useLayoutEffect mount C', + 'useEffect mount A', + 'useEffect mount B', + 'useEffect mount C', + ]); + } else { + expect(log).toEqual([ + 'useLayoutEffect mount A', + 'useLayoutEffect mount B', + 'useLayoutEffect mount C', + 'useEffect mount A', + 'useEffect mount B', + 'useEffect mount C', + ]); + } + + // Clear log after initial mount + log.length = 0; + + // Re-render with items reordered: [C, A, B] (move, not mount) + await act(() => { + ReactNoop.renderToRootWithID( + + + , + 'root', + ); + }); + + // Effects should NOT be double-invoked on reorder since these are + // moves, not new insertions. + expect(log).toEqual([]); + }); + // @gate __DEV__ it('should double invoke effects after a re-suspend', async () => { // Not using log.push because it silences double render logs. From e05c0621796fa3e2cd61d20693ea78559b74acab Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Fri, 27 Feb 2026 16:09:08 -0500 Subject: [PATCH 2/2] Add edge case tests for StrictMode move effects fix Add two tests that verify the StrictMode PlacementDEV fix handles mixed scenarios correctly: - Move + new mount: only the new item's effects are double-invoked - Move + removal: only the removed item's cleanup effects fire --- .../src/__tests__/StrictEffectsMode-test.js | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js index fadc32e2ec8d..dbef65de0ba8 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js @@ -889,6 +889,116 @@ describe('StrictEffectsMode', () => { expect(log).toEqual([]); }); + it('should only double-invoke effects for a new item when mixed with moves', async () => { + const log = []; + function Item({label}) { + React.useEffect(() => { + log.push('useEffect mount ' + label); + return () => log.push('useEffect unmount ' + label); + }, []); + + React.useLayoutEffect(() => { + log.push('useLayoutEffect mount ' + label); + return () => log.push('useLayoutEffect unmount ' + label); + }, []); + + return label; + } + + function App({items}) { + return items.map(item => ); + } + + // Initial mount: [A, B, C] + await act(() => { + ReactNoop.renderToRootWithID( + + + , + 'root', + ); + }); + + // Clear log after initial mount + log.length = 0; + + // Re-render with items reordered and a new item D: [C, A, D, B] + // C, A, B are moved (effects should NOT re-run) + // D is NEW (its effects SHOULD be double-invoked by StrictMode) + await act(() => { + ReactNoop.renderToRootWithID( + + + , + 'root', + ); + }); + + if (__DEV__) { + // Only D's effects should appear: mount, unmount, mount (StrictMode double-invoke) + expect(log).toEqual([ + 'useLayoutEffect mount D', + 'useEffect mount D', + 'useLayoutEffect unmount D', + 'useEffect unmount D', + 'useLayoutEffect mount D', + 'useEffect mount D', + ]); + } else { + // In production, D just mounts once + expect(log).toEqual(['useLayoutEffect mount D', 'useEffect mount D']); + } + }); + + it('should only fire unmount effects for a removed item when mixed with moves', async () => { + const log = []; + function Item({label}) { + React.useEffect(() => { + log.push('useEffect mount ' + label); + return () => log.push('useEffect unmount ' + label); + }, []); + + React.useLayoutEffect(() => { + log.push('useLayoutEffect mount ' + label); + return () => log.push('useLayoutEffect unmount ' + label); + }, []); + + return label; + } + + function App({items}) { + return items.map(item => ); + } + + // Initial mount: [A, B, C] + await act(() => { + ReactNoop.renderToRootWithID( + + + , + 'root', + ); + }); + + // Clear log after initial mount + log.length = 0; + + // Re-render with B removed and items reordered: [C, A] + // C, A are moved (effects should NOT re-run) + // B is REMOVED (its cleanup effects SHOULD fire) + await act(() => { + ReactNoop.renderToRootWithID( + + + , + 'root', + ); + }); + + // Only B's unmount effects should appear + expect(log).toEqual(['useLayoutEffect unmount B', 'useEffect unmount B']); + }); + // @gate __DEV__ it('should double invoke effects after a re-suspend', async () => { // Not using log.push because it silences double render logs.