Skip to content

Commit

Permalink
Add Placement bit without removing others (#20398)
Browse files Browse the repository at this point in the history
When scheduling a Placement effect, we should add the Placement bit
without resetting the others.

In the old fork, there are no flags to reset, anyway, since by the
time we reach the child reconciler, the flags will have already been
reset.

However, in the effects refactor, "static" flags are not reset, so this
can actually manifest as a bug. See #20285 for a regression test.
  • Loading branch information
acdlite committed Dec 7, 2020
1 parent 18d7574 commit 1377e46
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactChildFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,15 @@ function ChildReconciler(shouldTrackSideEffects) {
const oldIndex = current.index;
if (oldIndex < lastPlacedIndex) {
// This is a move.
newFiber.flags = Placement;
newFiber.flags |= Placement;
return lastPlacedIndex;
} else {
// This item can stay in place.
return oldIndex;
}
} else {
// This is an insertion.
newFiber.flags = Placement;
newFiber.flags |= Placement;
return lastPlacedIndex;
}
}
Expand All @@ -359,7 +359,7 @@ function ChildReconciler(shouldTrackSideEffects) {
// This is simpler for the single child case. We only need to do a
// placement for inserting new children.
if (shouldTrackSideEffects && newFiber.alternate === null) {
newFiber.flags = Placement;
newFiber.flags |= Placement;
}
return newFiber;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactChildFiber.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,15 @@ function ChildReconciler(shouldTrackSideEffects) {
const oldIndex = current.index;
if (oldIndex < lastPlacedIndex) {
// This is a move.
newFiber.flags = Placement;
newFiber.flags |= Placement;
return lastPlacedIndex;
} else {
// This item can stay in place.
return oldIndex;
}
} else {
// This is an insertion.
newFiber.flags = Placement;
newFiber.flags |= Placement;
return lastPlacedIndex;
}
}
Expand All @@ -359,7 +359,7 @@ function ChildReconciler(shouldTrackSideEffects) {
// This is simpler for the single child case. We only need to do a
// placement for inserting new children.
if (shouldTrackSideEffects && newFiber.alternate === null) {
newFiber.flags = Placement;
newFiber.flags |= Placement;
}
return newFiber;
}
Expand Down

0 comments on commit 1377e46

Please sign in to comment.