Skip to content

Commit 7f575fa

Browse files
author
Brian Vaughn
committed
Attempted refactoring of childLanes to be from workInProgress to parent
(rather than from children to workInProgress). This attempt isn't quite right though. Has some test failures that requires additional investigation.
1 parent 2b767a4 commit 7f575fa

File tree

4 files changed

+51
-26
lines changed

4 files changed

+51
-26
lines changed

packages/react-reconciler/src/ReactFiber.new.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
299299
// Static effects are not specific to a render.
300300
workInProgress.flags = current.flags & StaticMask;
301301
workInProgress.subtreeFlags = current.subtreeFlags & StaticMask;
302-
workInProgress.childLanes = current.childLanes;
302+
workInProgress.childLanes = NoLanes;
303303
workInProgress.lanes = current.lanes;
304304

305305
workInProgress.child = current.child;
@@ -388,7 +388,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
388388
}
389389
} else {
390390
// Reset to the cloned values that createWorkInProgress would've.
391-
workInProgress.childLanes = current.childLanes;
391+
workInProgress.childLanes = NoLanes;
392392
workInProgress.lanes = current.lanes;
393393

394394
workInProgress.child = current.child;

packages/react-reconciler/src/ReactFiberBeginWork.new.js

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,7 +1883,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
18831883
prevOffscreenState === null
18841884
? mountSuspenseOffscreenState(renderLanes)
18851885
: updateSuspenseOffscreenState(prevOffscreenState, renderLanes);
1886-
primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
1886+
primaryChildFragment.childLanes = workInProgress.childLanes = getRemainingWorkInPrimaryTree(
18871887
current,
18881888
renderLanes,
18891889
);
@@ -1920,7 +1920,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
19201920
prevOffscreenState === null
19211921
? mountSuspenseOffscreenState(renderLanes)
19221922
: updateSuspenseOffscreenState(prevOffscreenState, renderLanes);
1923-
primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
1923+
// TODO (effects) Document
1924+
primaryChildFragment.childLanes = workInProgress.childLanes = getRemainingWorkInPrimaryTree(
19241925
current,
19251926
renderLanes,
19261927
);
@@ -1989,6 +1990,18 @@ function mountSuspenseFallbackChildren(
19891990
primaryChildFragment.childLanes = NoLanes;
19901991
primaryChildFragment.pendingProps = primaryChildProps;
19911992

1993+
// TODO (effects) This is tricky because by the time we are resetting the childLanes here
1994+
// they have already bubbled to the parent SuspenseComponent and its parent also
1995+
// (during the previous complete phase bubbling back up to Suspense).
1996+
//
1997+
// Compare this to the old fork, where we will restart work on the Suspense node
1998+
// before bubbling higher (and letting the values leak out).
1999+
//
2000+
// We could reset childLanes for the parent SuspenseComponent safely,
2001+
// but we can't reset childLanes for its parent without potentially erasing sibling updates.
2002+
// Instead we need to prevent the childLanes from bubbling in the first place during a suspend.
2003+
workInProgress.childLanes = NoLanes;
2004+
19922005
if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
19932006
// Reset the durations from the first pass so they aren't included in the
19942007
// final amounts. This seems counterintuitive, since we're intentionally
@@ -2110,6 +2123,9 @@ function updateSuspenseFallbackChildren(
21102123
primaryChildFragment.childLanes = NoLanes;
21112124
primaryChildFragment.pendingProps = primaryChildProps;
21122125

2126+
// TODO (effects)
2127+
workInProgress.childLanes = NoLanes;
2128+
21132129
if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
21142130
// Reset the durations from the first pass so they aren't included in the
21152131
// final amounts. This seems counterintuitive, since we're intentionally
@@ -2969,7 +2985,7 @@ function bailoutOnAlreadyFinishedWork(
29692985
markSkippedUpdateLanes(workInProgress.lanes);
29702986

29712987
// Check if the children have any pending work.
2972-
if (!includesSomeLane(renderLanes, workInProgress.childLanes)) {
2988+
if (!includesSomeLane(renderLanes, current.childLanes)) {
29732989
// The children don't have any work either. We can skip them.
29742990
// TODO: Once we add back resuming, we should check if the children are
29752991
// a work-in-progress set. If so, we need to transfer their effects.
@@ -3118,7 +3134,7 @@ function beginWork(
31183134
// Profiler should only call onRender when one of its descendants actually rendered.
31193135
const hasChildWork = includesSomeLane(
31203136
renderLanes,
3121-
workInProgress.childLanes,
3137+
current.childLanes,
31223138
);
31233139
if (hasChildWork) {
31243140
workInProgress.flags |= Update;
@@ -3197,9 +3213,15 @@ function beginWork(
31973213
case SuspenseListComponent: {
31983214
const didSuspendBefore = (current.flags & DidCapture) !== NoFlags;
31993215

3216+
// TODO current.childLanes is right; workInProgress.childLanes is wrong.
3217+
// Presumably we bubbled something up that we shouldn't have then.
3218+
// Maybet his is a variation of the Suspense thing earlier?
3219+
// Either we need to reset (or not copy)?
3220+
// Or maybe we need to *set* or update in this case?
32003221
const hasChildWork = includesSomeLane(
32013222
renderLanes,
3202-
workInProgress.childLanes,
3223+
// workInProgress.childLanes,
3224+
current.childLanes, // This causes a loop for some tests
32033225
);
32043226

32053227
if (didSuspendBefore) {

packages/react-reconciler/src/ReactFiberCompleteWork.new.js

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ function cutOffTailIfNeeded(
684684

685685
function bubbleProperties(completedWork: Fiber): void {
686686
bubbleProfilerDurationsFromChildren(completedWork);
687-
bubbleLanesFromChildren(completedWork);
687+
bubbleLanesToParent(completedWork);
688688
bubbleFlagsToParent(completedWork);
689689
}
690690

@@ -707,20 +707,18 @@ function bubbleFlagsToParent(completedWork: Fiber): void {
707707
}
708708
}
709709

710+
// TODO (effects) Broken Suspense problem;
711+
// We aren't calling bubbleLanesToParent() for the Suspended components which means OffscreenLane doesn't bubble up.
712+
710713
// TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done.
711-
function bubbleLanesFromChildren(completedWork: Fiber): void {
712-
let newChildLanes = NoLanes;
713-
let child = completedWork.child;
714-
while (child !== null) {
715-
newChildLanes = mergeLanes(
716-
newChildLanes,
717-
mergeLanes(child.lanes, child.childLanes),
714+
function bubbleLanesToParent(completedWork: Fiber): void {
715+
const parent = completedWork.return;
716+
if (parent !== null) {
717+
parent.childLanes = mergeLanes(
718+
parent.childLanes,
719+
mergeLanes(completedWork.lanes, completedWork.childLanes),
718720
);
719-
720-
child = child.sibling;
721721
}
722-
723-
completedWork.childLanes = newChildLanes;
724722
}
725723

726724
// TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done.
@@ -1014,6 +1012,8 @@ function completeWork(
10141012
) {
10151013
transferActualDuration(workInProgress);
10161014
}
1015+
// TODO (effects) Comment about why
1016+
// bubbleProperties(workInProgress);
10171017
return workInProgress;
10181018
}
10191019

@@ -1204,6 +1204,7 @@ function completeWork(
12041204
ForceSuspenseFallback,
12051205
),
12061206
);
1207+
bubbleProperties(workInProgress);
12071208
return workInProgress.child;
12081209
}
12091210
row = row.sibling;
@@ -1332,6 +1333,7 @@ function completeWork(
13321333
suspenseContext = setDefaultShallowSuspenseContext(suspenseContext);
13331334
}
13341335
pushSuspenseContext(workInProgress, suspenseContext);
1336+
bubbleProperties(next);
13351337
// Do a pass over the next row.
13361338
return next;
13371339
}
@@ -1439,13 +1441,13 @@ function completeWork(
14391441
}
14401442

14411443
// Don't bubble properties for hidden children.
1442-
if (
1443-
!nextIsHidden ||
1444-
includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) ||
1445-
(workInProgress.mode & ConcurrentMode) === NoLanes
1446-
) {
1447-
bubbleProperties(workInProgress);
1448-
}
1444+
// if (
1445+
// !nextIsHidden ||
1446+
// includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) ||
1447+
// (workInProgress.mode & ConcurrentMode) === NoLanes
1448+
// ) {
1449+
bubbleProperties(workInProgress);
1450+
// }
14491451

14501452
return null;
14511453
}

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ import {
150150
SyncLane,
151151
SyncBatchedLane,
152152
NoTimestamp,
153+
OffscreenLane,
153154
findUpdateLane,
154155
findTransitionLane,
155156
findRetryLane,

0 commit comments

Comments
 (0)