From 7207a6541e11372ba6ac6f44621eda03c7f16598 Mon Sep 17 00:00:00 2001 From: Emmanuel Oppong Date: Tue, 24 Feb 2026 16:16:14 -0600 Subject: [PATCH 1/2] fix: handle Symbol values in hook dependency size change warning When a hook's dependency array changes size and contains Symbol values, the error message generation crashes with "Cannot convert a Symbol value to a string" because Array.join() implicitly calls toString() on each element. Use String() explicitly to safely convert any dep value. Fixes #19848 --- .../react-reconciler/src/ReactFiberHooks.js | 4 +-- .../src/__tests__/ReactHooks-test.internal.js | 27 +++++++++++++++++++ packages/react-server/src/ReactFizzHooks.js | 4 +-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 0450495ff0d4..8ea915b50072 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -483,8 +483,8 @@ function areHookInputsEqual( 'Previous: %s\n' + 'Incoming: %s', currentHookNameInDev, - `[${prevDeps.join(', ')}]`, - `[${nextDeps.join(', ')}]`, + `[${prevDeps.map(String).join(', ')}]`, + `[${nextDeps.map(String).join(', ')}]`, ); } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index e61e4a825602..e4e546f2674f 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -591,6 +591,33 @@ describe('ReactHooks', () => { ]); }); + it('warns about variable number of dependencies when deps contain Symbols', async () => { + const {useLayoutEffect} = React; + const sym = Symbol('testSymbol'); + function App(props) { + useLayoutEffect(() => {}, props.dependencies); + return null; + } + let root; + await act(() => { + root = ReactTestRenderer.create(, { + unstable_isConcurrent: true, + }); + }); + await act(() => { + root.update(); + }); + assertConsoleErrorDev([ + 'The final argument passed to useLayoutEffect changed size ' + + 'between renders. The order and size of this array must remain ' + + 'constant.\n' + + '\n' + + 'Previous: [Symbol(testSymbol)]\n' + + 'Incoming: [Symbol(testSymbol), extra]\n' + + ' in App (at **)', + ]); + }); + it('warns if switching from dependencies to no dependencies', async () => { const {useMemo} = React; function App({text, hasDeps}) { diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 24e676fc7108..d1d853f0c85f 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -151,8 +151,8 @@ function areHookInputsEqual( 'Previous: %s\n' + 'Incoming: %s', currentHookNameInDev, - `[${nextDeps.join(', ')}]`, - `[${prevDeps.join(', ')}]`, + `[${nextDeps.map(String).join(', ')}]`, + `[${prevDeps.map(String).join(', ')}]`, ); } } From a2b204085ad1fcfd8ea4e5cdd4615f9cab3158f5 Mon Sep 17 00:00:00 2001 From: Emmanuel Oppong Date: Thu, 26 Feb 2026 12:20:13 -0600 Subject: [PATCH 2/2] fix: don't double-invoke effects for moved children in StrictMode In StrictMode, `doubleInvokeEffectsInDEVIfNecessary` fires for any fiber that has the `PlacementDEV` flag set. Previously, `placeChild` set `PlacementDEV` on both newly inserted fibers AND fibers that were moved to a different position in an array. This caused a regression in React 19: when a keyed child is reordered within an array, its effects are re-run in dev/StrictMode even when dependencies are empty `[]`. The same component in production, or in React 18, correctly skips effect re-runs for moves. The fix is to only set `PlacementDEV` for actual insertions (where `current === null`). Moved fibers (`current !== null`, `oldIndex < lastPlacedIndex`) still receive the `Placement` flag so the host node is correctly repositioned in the DOM, but StrictMode no longer treats them as new mounts. Also fix unused `ref3` variable in ReactFabric-test.internal.js introduced in #35912 (copy-paste: third View used ref2 instead of ref3). Fixes #32561 --- .../__tests__/ReactFabric-test.internal.js | 2 +- .../react-reconciler/src/ReactChildFiber.js | 7 ++- .../src/__tests__/StrictEffectsMode-test.js | 48 +++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 5d0c8b8b9e97..e04ff2b550c3 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -1215,7 +1215,7 @@ describe('ReactFabric', () => { }} /> { expect(event.timeStamp).toBe(explicitTimeStampLowerCase); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 42f1b70918d3..8b41dc4106f2 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -524,8 +524,11 @@ function createChildReconciler( if (current !== null) { const oldIndex = current.index; if (oldIndex < lastPlacedIndex) { - // This is a move. - newFiber.flags |= Placement | PlacementDEV; + // This is a move. The fiber already exists and is being repositioned in + // the DOM. We set Placement so the host node is moved, but we do NOT + // set PlacementDEV because the component is not newly mounting — + // StrictMode should not double-invoke effects for moved components. + 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..dc1f98a4d75a 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js @@ -964,4 +964,52 @@ describe('StrictEffectsMode', () => { 'Child dep create', ]); }); + + it('should not double invoke effects when a keyed child is moved in an array', async () => { + const log = []; + + function Item({id}) { + React.useEffect(() => { + log.push(`${id} effect mount`); + return () => log.push(`${id} effect unmount`); + }, []); + + React.useLayoutEffect(() => { + log.push(`${id} layout mount`); + return () => log.push(`${id} layout unmount`); + }, []); + + return id; + } + + const root = ReactNoop.createRoot(); + + // Initial render: [A, B, C] + await act(() => { + root.render( + + {['A', 'B', 'C'].map(id => ( + + ))} + , + ); + }); + + log.length = 0; // clear mount logs, only care about what happens on reorder + + // Reorder to [C, A, B] — all elements move but none are new + await act(() => { + root.render( + + {['C', 'A', 'B'].map(id => ( + + ))} + , + ); + }); + + // Moved elements should not have their effects re-run. + // Only DOM placement happens; no mount/unmount of effects. + expect(log).toEqual([]); + }); });