Skip to content

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

Open
everettbu wants to merge 2 commits into
mainfrom
rh/strict-mode-list
Open

[Bugfix] Don't double invoke effects for list reorders #652
everettbu wants to merge 2 commits into
mainfrom
rh/strict-mode-list

Conversation

@everettbu

@everettbu everettbu commented Mar 3, 2026

Copy link
Copy Markdown

Mirror of facebook/react#35952
Original author: rickhanlonii


Fixes react/react#32561

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).
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
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 3, 2026
@greptile-apps

greptile-apps Bot commented Mar 3, 2026

Copy link
Copy Markdown

Greptile Summary

Fixes a bug where StrictMode incorrectly double-invoked effects on keyed list reorders. The root cause was that placeChild in ReactChildFiber.js set the PlacementDEV flag for moved children (reorders), not just new insertions. Since ReactFiberWorkLoop.js uses PlacementDEV to decide which fibers get their effects double-invoked, this caused spurious unmount/remount cycles on reorder. The fix removes PlacementDEV from the move code path while preserving it for genuine insertions.

  • Removed PlacementDEV from the move branch in placeChild, keeping only the Placement flag needed for DOM reordering
  • Added three edge-case tests covering pure reorders, reorders mixed with new insertions, and reorders mixed with removals

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, targeted bugfix with comprehensive test coverage.
  • The change is a single-line removal of one flag from one code path, with clear semantic justification: moves are not insertions and should not trigger StrictMode double-invoke. The fix is well-supported by three new tests covering key edge cases. The PlacementDEV flag remains correctly applied for genuine insertions and single-child placements. No regressions are expected.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/react-reconciler/src/ReactChildFiber.js Removes PlacementDEV flag from the move branch of placeChild, so moved keyed children no longer trigger StrictMode double-invoke of effects. Insertions (line 536) and single child placements (line 545) correctly retain PlacementDEV.
packages/react-reconciler/src/tests/StrictEffectsMode-test.js Adds three well-structured edge-case tests: pure reorder (no effect re-fire), reorder with a new insertion (only new item double-invoked), and reorder with a removal (only removed item's cleanup fires).

Last reviewed commit: e05c062

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: StrictMode reruns effects when a child is moved in an array

2 participants