Skip to content

[Bugfix] Don't double invoke effects for list reorders #35952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rickhanlonii wants to merge 2 commits into
base: main
Choose a base branch
from
+192 −1
Open

[Bugfix] Don't double invoke effects for list reorders #35952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f961eca
Fix StrictMode double-invoking effects on keyed child reorder
rickhanlonii Feb 27, 2026
e05c062
Add edge case tests for StrictMode move effects fix
rickhanlonii Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactChildFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
191 changes: 191 additions & 0 deletions packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,197 @@ 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 => <Item key={item} label={item} />);
}

// Initial mount: [A, B, C]
await act(() => {
ReactNoop.renderToRootWithID(
<React.StrictMode>
<App items={['A', 'B', 'C']} />
</React.StrictMode>,
'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(
<React.StrictMode>
<App items={['C', 'A', 'B']} />
</React.StrictMode>,
'root',
);
});

// Effects should NOT be double-invoked on reorder since these are
// moves, not new insertions.
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 => <Item key={item} label={item} />);
}

// Initial mount: [A, B, C]
await act(() => {
ReactNoop.renderToRootWithID(
<React.StrictMode>
<App items={['A', 'B', 'C']} />
</React.StrictMode>,
'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(
<React.StrictMode>
<App items={['C', 'A', 'D', 'B']} />
</React.StrictMode>,
'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 => <Item key={item} label={item} />);
}

// Initial mount: [A, B, C]
await act(() => {
ReactNoop.renderToRootWithID(
<React.StrictMode>
<App items={['A', 'B', 'C']} />
</React.StrictMode>,
'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(
<React.StrictMode>
<App items={['C', 'A']} />
</React.StrictMode>,
'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.
Expand Down
Loading