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.

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

Bugfix: Revealing a hidden update #24685

Merged
merged 3 commits into from
Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,9 @@ export function createFiberFromOffscreen(
const fiber = createFiber(OffscreenComponent, pendingProps, key, mode);
fiber.elementType = REACT_OFFSCREEN_TYPE;
fiber.lanes = lanes;
const primaryChildInstance: OffscreenInstance = {};
const primaryChildInstance: OffscreenInstance = {
isHidden: false,
};
fiber.stateNode = primaryChildInstance;
return fiber;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/react-reconciler/src/ReactFiber.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,9 @@ export function createFiberFromOffscreen(
const fiber = createFiber(OffscreenComponent, pendingProps, key, mode);
fiber.elementType = REACT_OFFSCREEN_TYPE;
fiber.lanes = lanes;
const primaryChildInstance: OffscreenInstance = {};
const primaryChildInstance: OffscreenInstance = {
isHidden: false,
};
fiber.stateNode = primaryChildInstance;
return fiber;
}
Expand Down
21 changes: 19 additions & 2 deletions packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ import type {Lanes, Lane} from './ReactFiberLane.new';
import {
NoLane,
NoLanes,
OffscreenLane,
isSubsetOfLanes,
mergeLanes,
removeLanes,
isTransitionLane,
intersectLanes,
markRootEntangled,
Expand All @@ -108,6 +110,7 @@ import {StrictLegacyMode} from './ReactTypeOfMode';
import {
markSkippedUpdateLanes,
isUnsafeClassRenderPhaseUpdate,
getWorkInProgressRootRenderLanes,
} from './ReactFiberWorkLoop.new';
import {
enqueueConcurrentClassUpdate,
Expand Down Expand Up @@ -523,9 +526,23 @@ export function processUpdateQueue<State>(

let update = firstBaseUpdate;
do {
const updateLane = update.lane;
// TODO: Don't need this field anymore
const updateEventTime = update.eventTime;
if (!isSubsetOfLanes(renderLanes, updateLane)) {

// An extra OffscreenLane bit is added to updates that were made to
// a hidden tree, so that we can distinguish them from updates that were
// already there when the tree was hidden.
const updateLane = removeLanes(update.lane, OffscreenLane);
const isHiddenUpdate = updateLane !== update.lane;

// Check if this update was made while the tree was hidden. If so, then
// it's not a "base" update and we should disregard the extra base lanes
// that were added to renderLanes when we entered the Offscreen tree.
const shouldSkipUpdate = isHiddenUpdate
? !isSubsetOfLanes(getWorkInProgressRootRenderLanes(), updateLane)
: !isSubsetOfLanes(renderLanes, updateLane);

if (shouldSkipUpdate) {
// Priority is insufficient. Skip this update. If this is the first
// skipped update, the previous update/state is the new base
// update/state.
Expand Down
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2309,8 +2309,14 @@ function commitMutationEffectsOnFiber(
const offscreenFiber: Fiber = (finishedWork.child: any);

if (offscreenFiber.flags & Visibility) {
const offscreenInstance: OffscreenInstance = offscreenFiber.stateNode;
const newState: OffscreenState | null = offscreenFiber.memoizedState;
const isHidden = newState !== null;

// Track the current state on the Offscreen instance so we can
// read it during an event
offscreenInstance.isHidden = isHidden;

if (isHidden) {
const wasHidden =
offscreenFiber.alternate !== null &&
Expand Down Expand Up @@ -2354,10 +2360,15 @@ function commitMutationEffectsOnFiber(
commitReconciliationEffects(finishedWork);

if (flags & Visibility) {
const offscreenInstance: OffscreenInstance = finishedWork.stateNode;
const newState: OffscreenState | null = finishedWork.memoizedState;
const isHidden = newState !== null;
const offscreenBoundary: Fiber = finishedWork;

// Track the current state on the Offscreen instance so we can
// read it during an event
offscreenInstance.isHidden = isHidden;

if (enableSuspenseLayoutEffectSemantics) {
if (isHidden) {
if (!wasHidden) {
Expand Down
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2309,8 +2309,14 @@ function commitMutationEffectsOnFiber(
const offscreenFiber: Fiber = (finishedWork.child: any);

if (offscreenFiber.flags & Visibility) {
const offscreenInstance: OffscreenInstance = offscreenFiber.stateNode;
const newState: OffscreenState | null = offscreenFiber.memoizedState;
const isHidden = newState !== null;

// Track the current state on the Offscreen instance so we can
// read it during an event
offscreenInstance.isHidden = isHidden;

if (isHidden) {
const wasHidden =
offscreenFiber.alternate !== null &&
Expand Down Expand Up @@ -2354,10 +2360,15 @@ function commitMutationEffectsOnFiber(
commitReconciliationEffects(finishedWork);

if (flags & Visibility) {
const offscreenInstance: OffscreenInstance = finishedWork.stateNode;
const newState: OffscreenState | null = finishedWork.memoizedState;
const isHidden = newState !== null;
const offscreenBoundary: Fiber = finishedWork;

// Track the current state on the Offscreen instance so we can
// read it during an event
offscreenInstance.isHidden = isHidden;

if (enableSuspenseLayoutEffectSemantics) {
if (isHidden) {
if (!wasHidden) {
Expand Down
52 changes: 42 additions & 10 deletions packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,21 @@ import type {
Update as ClassUpdate,
} from './ReactFiberClassUpdateQueue.new';
import type {Lane, Lanes} from './ReactFiberLane.new';
import type {OffscreenInstance} from './ReactFiberOffscreenComponent';

import {warnAboutUpdateOnNotYetMountedFiberInDEV} from './ReactFiberWorkLoop.new';
import {NoLane, NoLanes, mergeLanes} from './ReactFiberLane.new';
import {
NoLane,
NoLanes,
mergeLanes,
markHiddenUpdate,
} from './ReactFiberLane.new';
import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
import {HostRoot} from './ReactWorkTags';
import {HostRoot, OffscreenComponent} from './ReactWorkTags';

type ConcurrentUpdate = {
export type ConcurrentUpdate = {
next: ConcurrentUpdate,
lane: Lane,
};

type ConcurrentQueue = {
Expand All @@ -38,11 +45,13 @@ type ConcurrentQueue = {
const concurrentQueues: Array<any> = [];
let concurrentQueuesIndex = 0;

export function finishQueueingConcurrentUpdates(): Lanes {
let concurrentlyUpdatedLanes: Lanes = NoLanes;

export function finishQueueingConcurrentUpdates(): void {
const endIndex = concurrentQueuesIndex;
concurrentQueuesIndex = 0;

let lanes = NoLanes;
concurrentlyUpdatedLanes = NoLanes;

let i = 0;
while (i < endIndex) {
Expand All @@ -68,12 +77,13 @@ export function finishQueueingConcurrentUpdates(): Lanes {
}

if (lane !== NoLane) {
lanes = mergeLanes(lanes, lane);
markUpdateLaneFromFiberToRoot(fiber, lane);
markUpdateLaneFromFiberToRoot(fiber, update, lane);
}
}
}

return lanes;
export function getConcurrentlyUpdatedLanes(): Lanes {
return concurrentlyUpdatedLanes;
}

function enqueueUpdate(
Expand All @@ -89,6 +99,8 @@ function enqueueUpdate(
concurrentQueues[concurrentQueuesIndex++] = update;
concurrentQueues[concurrentQueuesIndex++] = lane;

concurrentlyUpdatedLanes = mergeLanes(concurrentlyUpdatedLanes, lane);

// The fiber's `lane` field is used in some places to check if any work is
// scheduled, to perform an eager bailout, so we need to update it immediately.
// TODO: We should probably move this to the "shared" queue instead.
Expand Down Expand Up @@ -151,27 +163,47 @@ export function unsafe_markUpdateLaneFromFiberToRoot(
sourceFiber: Fiber,
lane: Lane,
): FiberRoot | null {
markUpdateLaneFromFiberToRoot(sourceFiber, lane);
markUpdateLaneFromFiberToRoot(sourceFiber, null, lane);
return getRootForUpdatedFiber(sourceFiber);
}

function markUpdateLaneFromFiberToRoot(sourceFiber: Fiber, lane: Lane): void {
function markUpdateLaneFromFiberToRoot(
sourceFiber: Fiber,
update: ConcurrentUpdate | null,
lane: Lane,
): void {
// Update the source fiber's lanes
sourceFiber.lanes = mergeLanes(sourceFiber.lanes, lane);
let alternate = sourceFiber.alternate;
if (alternate !== null) {
alternate.lanes = mergeLanes(alternate.lanes, lane);
}
// Walk the parent path to the root and update the child lanes.
let isHidden = false;
let parent = sourceFiber.return;
let node = sourceFiber;
while (parent !== null) {
parent.childLanes = mergeLanes(parent.childLanes, lane);
alternate = parent.alternate;
if (alternate !== null) {
alternate.childLanes = mergeLanes(alternate.childLanes, lane);
}

if (parent.tag === OffscreenComponent) {
const offscreenInstance: OffscreenInstance = parent.stateNode;
if (offscreenInstance.isHidden) {
isHidden = true;
}
}

node = parent;
parent = parent.return;
}

if (isHidden && update !== null && node.tag === HostRoot) {
const root: FiberRoot = node.stateNode;
markHiddenUpdate(root, update, lane);
}
}

function getRootForUpdatedFiber(sourceFiber: Fiber): FiberRoot | null {
Expand Down
18 changes: 16 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
import {
NoLane,
SyncLane,
OffscreenLane,
NoLanes,
isSubsetOfLanes,
includesBlockingLane,
Expand Down Expand Up @@ -83,6 +84,7 @@ import {
} from './ReactHookEffectTags';
import {
getWorkInProgressRoot,
getWorkInProgressRootRenderLanes,
scheduleUpdateOnFiber,
requestUpdateLane,
requestEventTime,
Expand Down Expand Up @@ -811,8 +813,20 @@ function updateReducer<S, I, A>(
let newBaseQueueLast = null;
let update = first;
do {
const updateLane = update.lane;
if (!isSubsetOfLanes(renderLanes, updateLane)) {
// An extra OffscreenLane bit is added to updates that were made to
// a hidden tree, so that we can distinguish them from updates that were
// already there when the tree was hidden.
const updateLane = removeLanes(update.lane, OffscreenLane);
const isHiddenUpdate = updateLane !== update.lane;

// Check if this update was made while the tree was hidden. If so, then
// it's not a "base" update and we should disregard the extra base lanes
// that were added to renderLanes when we entered the Offscreen tree.
const shouldSkipUpdate = isHiddenUpdate
? !isSubsetOfLanes(getWorkInProgressRootRenderLanes(), updateLane)
: !isSubsetOfLanes(renderLanes, updateLane);

if (shouldSkipUpdate) {
// Priority is insufficient. Skip this update. If this is the first
// skipped update, the previous update/state is the new base
// update/state.
Expand Down
33 changes: 33 additions & 0 deletions packages/react-reconciler/src/ReactFiberLane.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import type {FiberRoot} from './ReactInternalTypes';
import type {Transition} from './ReactFiberTracingMarkerComponent.new';
import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates.new';

// TODO: Ideally these types would be opaque but that doesn't work well with
// our reconciler fork infra, since these leak into non-reconciler packages.
Expand Down Expand Up @@ -648,6 +649,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
const entanglements = root.entanglements;
const eventTimes = root.eventTimes;
const expirationTimes = root.expirationTimes;
const hiddenUpdates = root.hiddenUpdates;

// Clear the lanes that no longer have pending work
let lanes = noLongerPendingLanes;
Expand All @@ -659,6 +661,21 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
eventTimes[index] = NoTimestamp;
expirationTimes[index] = NoTimestamp;

const hiddenUpdatesForLane = hiddenUpdates[index];
if (hiddenUpdatesForLane !== null) {
hiddenUpdates[index] = null;
// "Hidden" updates are updates that were made to a hidden component. They
// have special logic associated with them because they may be entangled
// with updates that occur outside that tree. But once the outer tree
// commits, they behave like regular updates.
for (let i = 0; i < hiddenUpdatesForLane.length; i++) {
const update = hiddenUpdatesForLane[i];
if (update !== null) {
update.lane &= ~OffscreenLane;
}
}
}

lanes &= ~lane;
}
}
Expand Down Expand Up @@ -694,6 +711,22 @@ export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) {
}
}

export function markHiddenUpdate(
root: FiberRoot,
update: ConcurrentUpdate,
lane: Lane,
) {
const index = laneToIndex(lane);
const hiddenUpdates = root.hiddenUpdates;
const hiddenUpdatesForLane = hiddenUpdates[index];
if (hiddenUpdatesForLane === null) {
hiddenUpdates[index] = [update];
} else {
hiddenUpdatesForLane.push(update);
}
update.lane = lane | OffscreenLane;
}

export function getBumpedLaneForHydration(
root: FiberRoot,
renderLanes: Lanes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ export type OffscreenQueue = {|
transitions: Array<Transition> | null,
|} | null;

export type OffscreenInstance = {};
export type OffscreenInstance = {|
isHidden: boolean,
|};
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberRoot.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ function FiberRootNode(
this.entangledLanes = NoLanes;
this.entanglements = createLaneMap(NoLanes);

this.hiddenUpdates = createLaneMap(null);

this.identifierPrefix = identifierPrefix;
this.onRecoverableError = onRecoverableError;

Expand Down
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ import {
import {
enqueueConcurrentRenderForLane,
finishQueueingConcurrentUpdates,
getConcurrentlyUpdatedLanes,
} from './ReactFiberConcurrentUpdates.new';

import {
Expand Down Expand Up @@ -425,6 +426,10 @@ export function getWorkInProgressRoot(): FiberRoot | null {
return workInProgressRoot;
}

export function getWorkInProgressRootRenderLanes(): Lanes {
return workInProgressRootRenderLanes;
}

export function requestEventTime() {
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
// We're inside React, so it's fine to read the actual time.
Expand Down Expand Up @@ -2059,7 +2064,7 @@ function commitRootImpl(

// Make sure to account for lanes that were updated by a concurrent event
// during the render phase; don't mark them as finished.
const concurrentlyUpdatedLanes = finishQueueingConcurrentUpdates();
const concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes();
remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes);

markRootFinished(root, remainingLanes);
Expand Down
Loading