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/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-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([]); + }); }); 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(', ')}]`, ); } }