Skip to content
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

[draft] Move resetChildLanes into complete work #19801

Closed
wants to merge 4 commits into from
Closed

[draft] Move resetChildLanes into complete work #19801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a789939
Moved resetChildLanes into complete work
Sep 8, 2020
0247a8a
Temporarily split lanes, flags, and profiler durations into separate …
Sep 9, 2020
2b767a4
Refactored flag bubbling to be from child to parent
Sep 9, 2020
7f575fa
Attempted refactoring of childLanes to be from workInProgress to parent
Sep 14, 2020
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
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactFiber.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
workInProgress.type = current.type;

// We already have an alternate.
workInProgress.subtreeFlags = NoFlags;
workInProgress.deletions = null;

if (enableProfilerTimer) {
Expand All @@ -299,7 +298,8 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
// Reset all effects except static ones.
// Static effects are not specific to a render.
workInProgress.flags = current.flags & StaticMask;
workInProgress.childLanes = current.childLanes;
workInProgress.subtreeFlags = current.subtreeFlags & StaticMask;
workInProgress.childLanes = NoLanes;
workInProgress.lanes = current.lanes;

workInProgress.child = current.child;
Expand Down Expand Up @@ -388,7 +388,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
}
} else {
// Reset to the cloned values that createWorkInProgress would've.
workInProgress.childLanes = current.childLanes;
workInProgress.childLanes = NoLanes;
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
workInProgress.lanes = current.lanes;

workInProgress.child = current.child;
Expand Down
32 changes: 27 additions & 5 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1883,7 +1883,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
prevOffscreenState === null
? mountSuspenseOffscreenState(renderLanes)
: updateSuspenseOffscreenState(prevOffscreenState, renderLanes);
primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
primaryChildFragment.childLanes = workInProgress.childLanes = getRemainingWorkInPrimaryTree(
current,
renderLanes,
);
Expand Down Expand Up @@ -1920,7 +1920,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
prevOffscreenState === null
? mountSuspenseOffscreenState(renderLanes)
: updateSuspenseOffscreenState(prevOffscreenState, renderLanes);
primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
// TODO (effects) Document
primaryChildFragment.childLanes = workInProgress.childLanes = getRemainingWorkInPrimaryTree(
current,
renderLanes,
);
Expand Down Expand Up @@ -1989,6 +1990,18 @@ function mountSuspenseFallbackChildren(
primaryChildFragment.childLanes = NoLanes;
primaryChildFragment.pendingProps = primaryChildProps;

// TODO (effects) This is tricky because by the time we are resetting the childLanes here
// they have already bubbled to the parent SuspenseComponent and its parent also
// (during the previous complete phase bubbling back up to Suspense).
//
// Compare this to the old fork, where we will restart work on the Suspense node
// before bubbling higher (and letting the values leak out).
//
// We could reset childLanes for the parent SuspenseComponent safely,
// but we can't reset childLanes for its parent without potentially erasing sibling updates.
// Instead we need to prevent the childLanes from bubbling in the first place during a suspend.
workInProgress.childLanes = NoLanes;

if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
// Reset the durations from the first pass so they aren't included in the
// final amounts. This seems counterintuitive, since we're intentionally
Expand Down Expand Up @@ -2110,6 +2123,9 @@ function updateSuspenseFallbackChildren(
primaryChildFragment.childLanes = NoLanes;
primaryChildFragment.pendingProps = primaryChildProps;

// TODO (effects)
workInProgress.childLanes = NoLanes;

if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
// Reset the durations from the first pass so they aren't included in the
// final amounts. This seems counterintuitive, since we're intentionally
Expand Down Expand Up @@ -2969,7 +2985,7 @@ function bailoutOnAlreadyFinishedWork(
markSkippedUpdateLanes(workInProgress.lanes);

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

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

if (didSuspendBefore) {
Expand Down
Loading