Set return pointer when reusing current tree (#20212)
* Do not fix return pointers during commit phase In the commit phase, we should be able to assume that the `return` pointers in the just-completed tree are consistent. The render phase should be responsible for ensuring these are always correct. I've removed the `return` pointer assignments from the render phase traversal logic. This isn't all of them, only the ones added recently during the effects refactor. The other ones have been around longer so I'll leave those for a later clean up. This breaks a few SuspenseList tests; I'll fix in the next commit. * Set return pointer when reusing current tree We always set the return pointer on freshly cloned, work-in-progress fibers. However, we were neglecting to set them on trees that are reused from current. I fixed this in the same path of the complete phase where we reset the fiber flags. This is a code smell because it assumes the commit phase is never concurrent with the render phase. Our eventual goal is to make fibers a lock free data structure. Will address further during refactor to alternate model.
Andrew Clark committed
Nov 10, 2020 at 13:20 UTC
c896cf9617626be54f4ef599578e6f8a3a874205
2 files changed
+10
-14
packages/react-reconciler/src/ReactFiberCommitWork.new.js
-14
@@ -463,7 +463,6 @@ function iterativelyCommitBeforeMutationEffects_begin() {
463
(fiber.subtreeFlags & BeforeMutationMask) !== NoFlags &&
464
child !== null
465
) {
466
- child.return = fiber;
466
nextEffect = child;
467
} else {
468
iterativelyCommitBeforeMutationEffects_complete();
@@ -497,7 +496,6 @@ function iterativelyCommitBeforeMutationEffects_complete() {
496
497
const sibling = fiber.sibling;
498
if (sibling !== null) {
500
- sibling.return = fiber.return;
499
nextEffect = sibling;
500
return;
501
}
@@ -715,7 +713,6 @@ function iterativelyCommitMutationEffects_begin(
713
714
const child = fiber.child;
715
if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
718
- child.return = fiber;
716
nextEffect = child;
717
} else {
718
iterativelyCommitMutationEffects_complete(root, renderPriorityLevel);
@@ -754,7 +751,6 @@ function iterativelyCommitMutationEffects_complete(
751
752
const sibling = fiber.sibling;
753
if (sibling !== null) {
757
- sibling.return = fiber.return;
754
nextEffect = sibling;
755
return;
756
}
@@ -1176,14 +1172,12 @@ function iterativelyCommitLayoutEffects_begin(
1172
}
1173
const sibling = finishedWork.sibling;
1174
if (sibling !== null) {
1179
- sibling.return = finishedWork.return;
1175
nextEffect = sibling;
1176
} else {
1177
nextEffect = finishedWork.return;
1178
iterativelyCommitLayoutEffects_complete(subtreeRoot, finishedRoot);
1179
}
1180
} else {
1186
- firstChild.return = finishedWork;
1181
nextEffect = firstChild;
1182
}
1183
} else {
@@ -1230,7 +1224,6 @@ function iterativelyCommitLayoutEffects_complete(
1224
1225
const sibling = fiber.sibling;
1226
if (sibling !== null) {
1233
- sibling.return = fiber.return;
1227
nextEffect = sibling;
1228
return;
1229
}
@@ -1764,14 +1757,12 @@ function iterativelyCommitPassiveMountEffects_begin(
1757
}
1758
const sibling = fiber.sibling;
1759
if (sibling !== null) {
1767
- sibling.return = fiber.return;
1760
nextEffect = sibling;
1761
} else {
1762
nextEffect = fiber.return;
1763
iterativelyCommitPassiveMountEffects_complete(subtreeRoot, root);
1764
}
1765
} else {
1774
- firstChild.return = fiber;
1766
nextEffect = firstChild;
1767
}
1768
} else {
@@ -1817,7 +1808,6 @@ function iterativelyCommitPassiveMountEffects_complete(
1808
1809
const sibling = fiber.sibling;
1810
if (sibling !== null) {
1820
- sibling.return = fiber.return;
1811
nextEffect = sibling;
1812
return;
1813
}
@@ -1896,7 +1886,6 @@ function iterativelyCommitPassiveUnmountEffects_begin() {
1886
}
1887
1888
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
1899
- child.return = fiber;
1889
nextEffect = child;
1890
} else {
1891
iterativelyCommitPassiveUnmountEffects_complete();
@@ -1915,7 +1904,6 @@ function iterativelyCommitPassiveUnmountEffects_complete() {
1904
1905
const sibling = fiber.sibling;
1906
if (sibling !== null) {
1918
- sibling.return = fiber.return;
1907
nextEffect = sibling;
1908
return;
1909
}
@@ -1953,7 +1941,6 @@ function iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_begin(
1941
const fiber = nextEffect;
1942
const child = fiber.child;
1943
if ((fiber.subtreeFlags & PassiveStatic) !== NoFlags && child !== null) {
1956
- child.return = fiber;
1944
nextEffect = child;
1945
} else {
1946
iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_complete(
@@ -1981,7 +1968,6 @@ function iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_complete(
1968
1969
const sibling = fiber.sibling;
1970
if (sibling !== null) {
1984
- sibling.return = fiber.return;
1971
nextEffect = sibling;
1972
return;
1973
}
packages/react-reconciler/src/ReactFiberCompleteWork.new.js
+10
@@ -738,6 +738,11 @@ function bubbleProperties(completedWork: Fiber) {
738
subtreeFlags |= child.subtreeFlags;
739
subtreeFlags |= child.flags;
740
741
+ // Update the return pointer so the tree is consistent. This is a code
742
+ // smell because it assumes the commit phase is never concurrent with
743
+ // the render phase. Will address during refactor to alternate model.
744
+ child.return = completedWork;
745
+
746
child = child.sibling;
747
}
748
}
@@ -784,6 +789,11 @@ function bubbleProperties(completedWork: Fiber) {
789
subtreeFlags |= child.subtreeFlags & StaticMask;
790
subtreeFlags |= child.flags & StaticMask;
791
792
+ // Update the return pointer so the tree is consistent. This is a code
793
+ // smell because it assumes the commit phase is never concurrent with
794
+ // the render phase. Will address during refactor to alternate model.
795
+ child.return = completedWork;
796
+
797
child = child.sibling;
798
}
799
}