@samitouri / QOS-React-2 / commits / 79f54c16dc

Bugfix: Revealing a hidden update (#24685)

* Add `isHidden` to OffscreenInstance We need to be able to read whether an offscreen tree is hidden from an imperative event. We can store this on its OffscreenInstance. We were already scheduling a commit effect whenever the visibility changes, in order to toggle the inner effects. So we can reuse that. * [FORKED] Bugfix: Revealing a hidden update This fixes a bug I discovered related to revealing a hidden Offscreen tree. When this happens, we include in that render all the updates that had previously been deferred — that is, all the updates that would have already committed if the tree weren't hidden. This is necessary to avoid tearing with the surrounding contents. (This was the "flickering" Suspense bug we found a few years ago: #18411.) The way we do this is by tracking the lanes of the updates that were deferred by a hidden tree. These are the "base" lanes. Then, in order to reveal the hidden tree, we process any update that matches one of those base lanes. The bug I discovered is that some of these base lanes may include updates that were not present at the time the tree was hidden. We cannot flush those updates earlier that the surrounding contents — that, too, could cause tearing. The crux of the problem is that we sometimes reuse the same lane for base updates and for non-base updates. So the lane alone isn't sufficient to distinguish between these cases. We must track this in some other way. The solution I landed upon was to add an extra OffscreenLane bit to any update that is made to a hidden tree. Then later when we reveal the tree, we'll know not to treat them as base updates. The extra OffscreenLane bit is removed as soon as that lane is committed by the root (markRootFinished) — at that point, it gets "upgraded" to a base update. The trickiest part of this algorithm is reliably detecting when an update is made to a hidden tree. What makes this challenging is when the update is received during a concurrent event, while a render is already in progress — it's possible the work-in-progress render is about to flip the visibility of the tree that's being updated, leading to a race condition. To avoid a race condition, we will wait to read the visibility of the tree until the current render has finished. In other words, this makes it an atomic operation. Most of this logic was already implemented in #24663. Because this bugfix depends on a moderately risky refactor to the update queue (#24663), it only works in the "new" reconciler fork. We will roll it out gradually to www before landing in the main fork. * Add previous commit to list of forked revisions

Andrew Clark committed Jun 7, 2022 at 20:04 UTC 79f54c16dc3d5298e6037df75db2beb3552896e9
15 files changed +520 -18
packages/react-reconciler/src/ReactFiber.new.js
+3 -1
@@ -715,7 +715,9 @@ export function createFiberFromOffscreen(
715 const fiber = createFiber(OffscreenComponent, pendingProps, key, mode);
716 fiber.elementType = REACT_OFFSCREEN_TYPE;
717 fiber.lanes = lanes;
718 - const primaryChildInstance: OffscreenInstance = {};
718 + const primaryChildInstance: OffscreenInstance = {
719 + isHidden: false,
720 + };
721 fiber.stateNode = primaryChildInstance;
722 return fiber;
723 }
packages/react-reconciler/src/ReactFiber.old.js
+3 -1
@@ -715,7 +715,9 @@ export function createFiberFromOffscreen(
715 const fiber = createFiber(OffscreenComponent, pendingProps, key, mode);
716 fiber.elementType = REACT_OFFSCREEN_TYPE;
717 fiber.lanes = lanes;
718 - const primaryChildInstance: OffscreenInstance = {};
718 + const primaryChildInstance: OffscreenInstance = {
719 + isHidden: false,
720 + };
721 fiber.stateNode = primaryChildInstance;
722 return fiber;
723 }
packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js
+19 -2
@@ -90,8 +90,10 @@ import type {Lanes, Lane} from './ReactFiberLane.new';
90 import {
91 NoLane,
92 NoLanes,
93 + OffscreenLane,
94 isSubsetOfLanes,
95 mergeLanes,
96 + removeLanes,
97 isTransitionLane,
98 intersectLanes,
99 markRootEntangled,
@@ -108,6 +110,7 @@ import {StrictLegacyMode} from './ReactTypeOfMode';
110 import {
111 markSkippedUpdateLanes,
112 isUnsafeClassRenderPhaseUpdate,
113 + getWorkInProgressRootRenderLanes,
114 } from './ReactFiberWorkLoop.new';
115 import {
116 enqueueConcurrentClassUpdate,
@@ -523,9 +526,23 @@ export function processUpdateQueue<State>(
526
527 let update = firstBaseUpdate;
528 do {
526 - const updateLane = update.lane;
529 + // TODO: Don't need this field anymore
530 const updateEventTime = update.eventTime;
528 - if (!isSubsetOfLanes(renderLanes, updateLane)) {
531 +
532 + // An extra OffscreenLane bit is added to updates that were made to
533 + // a hidden tree, so that we can distinguish them from updates that were
534 + // already there when the tree was hidden.
535 + const updateLane = removeLanes(update.lane, OffscreenLane);
536 + const isHiddenUpdate = updateLane !== update.lane;
537 +
538 + // Check if this update was made while the tree was hidden. If so, then
539 + // it's not a "base" update and we should disregard the extra base lanes
540 + // that were added to renderLanes when we entered the Offscreen tree.
541 + const shouldSkipUpdate = isHiddenUpdate
542 + ? !isSubsetOfLanes(getWorkInProgressRootRenderLanes(), updateLane)
543 + : !isSubsetOfLanes(renderLanes, updateLane);
544 +
545 + if (shouldSkipUpdate) {
546 // Priority is insufficient. Skip this update. If this is the first
547 // skipped update, the previous update/state is the new base
548 // update/state.
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+11
@@ -2309,8 +2309,14 @@ function commitMutationEffectsOnFiber(
2309 const offscreenFiber: Fiber = (finishedWork.child: any);
2310
2311 if (offscreenFiber.flags & Visibility) {
2312 + const offscreenInstance: OffscreenInstance = offscreenFiber.stateNode;
2313 const newState: OffscreenState | null = offscreenFiber.memoizedState;
2314 const isHidden = newState !== null;
2315 +
2316 + // Track the current state on the Offscreen instance so we can
2317 + // read it during an event
2318 + offscreenInstance.isHidden = isHidden;
2319 +
2320 if (isHidden) {
2321 const wasHidden =
2322 offscreenFiber.alternate !== null &&
@@ -2354,10 +2360,15 @@ function commitMutationEffectsOnFiber(
2360 commitReconciliationEffects(finishedWork);
2361
2362 if (flags & Visibility) {
2363 + const offscreenInstance: OffscreenInstance = finishedWork.stateNode;
2364 const newState: OffscreenState | null = finishedWork.memoizedState;
2365 const isHidden = newState !== null;
2366 const offscreenBoundary: Fiber = finishedWork;
2367
2368 + // Track the current state on the Offscreen instance so we can
2369 + // read it during an event
2370 + offscreenInstance.isHidden = isHidden;
2371 +
2372 if (enableSuspenseLayoutEffectSemantics) {
2373 if (isHidden) {
2374 if (!wasHidden) {
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+11
@@ -2309,8 +2309,14 @@ function commitMutationEffectsOnFiber(
2309 const offscreenFiber: Fiber = (finishedWork.child: any);
2310
2311 if (offscreenFiber.flags & Visibility) {
2312 + const offscreenInstance: OffscreenInstance = offscreenFiber.stateNode;
2313 const newState: OffscreenState | null = offscreenFiber.memoizedState;
2314 const isHidden = newState !== null;
2315 +
2316 + // Track the current state on the Offscreen instance so we can
2317 + // read it during an event
2318 + offscreenInstance.isHidden = isHidden;
2319 +
2320 if (isHidden) {
2321 const wasHidden =
2322 offscreenFiber.alternate !== null &&
@@ -2354,10 +2360,15 @@ function commitMutationEffectsOnFiber(
2360 commitReconciliationEffects(finishedWork);
2361
2362 if (flags & Visibility) {
2363 + const offscreenInstance: OffscreenInstance = finishedWork.stateNode;
2364 const newState: OffscreenState | null = finishedWork.memoizedState;
2365 const isHidden = newState !== null;
2366 const offscreenBoundary: Fiber = finishedWork;
2367
2368 + // Track the current state on the Offscreen instance so we can
2369 + // read it during an event
2370 + offscreenInstance.isHidden = isHidden;
2371 +
2372 if (enableSuspenseLayoutEffectSemantics) {
2373 if (isHidden) {
2374 if (!wasHidden) {
packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js
+42 -10
@@ -17,14 +17,21 @@ import type {
17 Update as ClassUpdate,
18 } from './ReactFiberClassUpdateQueue.new';
19 import type {Lane, Lanes} from './ReactFiberLane.new';
20 +import type {OffscreenInstance} from './ReactFiberOffscreenComponent';
21
22 import {warnAboutUpdateOnNotYetMountedFiberInDEV} from './ReactFiberWorkLoop.new';
22 -import {NoLane, NoLanes, mergeLanes} from './ReactFiberLane.new';
23 +import {
24 + NoLane,
25 + NoLanes,
26 + mergeLanes,
27 + markHiddenUpdate,
28 +} from './ReactFiberLane.new';
29 import {NoFlags, Placement, Hydrating} from './ReactFiberFlags';
24 -import {HostRoot} from './ReactWorkTags';
30 +import {HostRoot, OffscreenComponent} from './ReactWorkTags';
31
26 -type ConcurrentUpdate = {
32 +export type ConcurrentUpdate = {
33 next: ConcurrentUpdate,
34 + lane: Lane,
35 };
36
37 type ConcurrentQueue = {
@@ -38,11 +45,13 @@ type ConcurrentQueue = {
45 const concurrentQueues: Array<any> = [];
46 let concurrentQueuesIndex = 0;
47
41 -export function finishQueueingConcurrentUpdates(): Lanes {
48 +let concurrentlyUpdatedLanes: Lanes = NoLanes;
49 +
50 +export function finishQueueingConcurrentUpdates(): void {
51 const endIndex = concurrentQueuesIndex;
52 concurrentQueuesIndex = 0;
53
45 - let lanes = NoLanes;
54 + concurrentlyUpdatedLanes = NoLanes;
55
56 let i = 0;
57 while (i < endIndex) {
@@ -68,12 +77,13 @@ export function finishQueueingConcurrentUpdates(): Lanes {
77 }
78
79 if (lane !== NoLane) {
71 - lanes = mergeLanes(lanes, lane);
72 - markUpdateLaneFromFiberToRoot(fiber, lane);
80 + markUpdateLaneFromFiberToRoot(fiber, update, lane);
81 }
82 }
83 +}
84
76 - return lanes;
85 +export function getConcurrentlyUpdatedLanes(): Lanes {
86 + return concurrentlyUpdatedLanes;
87 }
88
89 function enqueueUpdate(
@@ -89,6 +99,8 @@ function enqueueUpdate(
99 concurrentQueues[concurrentQueuesIndex++] = update;
100 concurrentQueues[concurrentQueuesIndex++] = lane;
101
102 + concurrentlyUpdatedLanes = mergeLanes(concurrentlyUpdatedLanes, lane);
103 +
104 // The fiber's `lane` field is used in some places to check if any work is
105 // scheduled, to perform an eager bailout, so we need to update it immediately.
106 // TODO: We should probably move this to the "shared" queue instead.
@@ -151,11 +163,15 @@ export function unsafe_markUpdateLaneFromFiberToRoot(
163 sourceFiber: Fiber,
164 lane: Lane,
165 ): FiberRoot | null {
154 - markUpdateLaneFromFiberToRoot(sourceFiber, lane);
166 + markUpdateLaneFromFiberToRoot(sourceFiber, null, lane);
167 return getRootForUpdatedFiber(sourceFiber);
168 }
169
158 -function markUpdateLaneFromFiberToRoot(sourceFiber: Fiber, lane: Lane): void {
170 +function markUpdateLaneFromFiberToRoot(
171 + sourceFiber: Fiber,
172 + update: ConcurrentUpdate | null,
173 + lane: Lane,
174 +): void {
175 // Update the source fiber's lanes
176 sourceFiber.lanes = mergeLanes(sourceFiber.lanes, lane);
177 let alternate = sourceFiber.alternate;
@@ -163,15 +179,31 @@ function markUpdateLaneFromFiberToRoot(sourceFiber: Fiber, lane: Lane): void {
179 alternate.lanes = mergeLanes(alternate.lanes, lane);
180 }
181 // Walk the parent path to the root and update the child lanes.
182 + let isHidden = false;
183 let parent = sourceFiber.return;
184 + let node = sourceFiber;
185 while (parent !== null) {
186 parent.childLanes = mergeLanes(parent.childLanes, lane);
187 alternate = parent.alternate;
188 if (alternate !== null) {
189 alternate.childLanes = mergeLanes(alternate.childLanes, lane);
190 }
191 +
192 + if (parent.tag === OffscreenComponent) {
193 + const offscreenInstance: OffscreenInstance = parent.stateNode;
194 + if (offscreenInstance.isHidden) {
195 + isHidden = true;
196 + }
197 + }
198 +
199 + node = parent;
200 parent = parent.return;
201 }
202 +
203 + if (isHidden && update !== null && node.tag === HostRoot) {
204 + const root: FiberRoot = node.stateNode;
205 + markHiddenUpdate(root, update, lane);
206 + }
207 }
208
209 function getRootForUpdatedFiber(sourceFiber: Fiber): FiberRoot | null {
packages/react-reconciler/src/ReactFiberHooks.new.js
+16 -2
@@ -44,6 +44,7 @@ import {
44 import {
45 NoLane,
46 SyncLane,
47 + OffscreenLane,
48 NoLanes,
49 isSubsetOfLanes,
50 includesBlockingLane,
@@ -83,6 +84,7 @@ import {
84 } from './ReactHookEffectTags';
85 import {
86 getWorkInProgressRoot,
87 + getWorkInProgressRootRenderLanes,
88 scheduleUpdateOnFiber,
89 requestUpdateLane,
90 requestEventTime,
@@ -811,8 +813,20 @@ function updateReducer<S, I, A>(
813 let newBaseQueueLast = null;
814 let update = first;
815 do {
814 - const updateLane = update.lane;
815 - if (!isSubsetOfLanes(renderLanes, updateLane)) {
816 + // An extra OffscreenLane bit is added to updates that were made to
817 + // a hidden tree, so that we can distinguish them from updates that were
818 + // already there when the tree was hidden.
819 + const updateLane = removeLanes(update.lane, OffscreenLane);
820 + const isHiddenUpdate = updateLane !== update.lane;
821 +
822 + // Check if this update was made while the tree was hidden. If so, then
823 + // it's not a "base" update and we should disregard the extra base lanes
824 + // that were added to renderLanes when we entered the Offscreen tree.
825 + const shouldSkipUpdate = isHiddenUpdate
826 + ? !isSubsetOfLanes(getWorkInProgressRootRenderLanes(), updateLane)
827 + : !isSubsetOfLanes(renderLanes, updateLane);
828 +
829 + if (shouldSkipUpdate) {
830 // Priority is insufficient. Skip this update. If this is the first
831 // skipped update, the previous update/state is the new base
832 // update/state.
packages/react-reconciler/src/ReactFiberLane.new.js
+33
@@ -9,6 +9,7 @@
9
10 import type {FiberRoot} from './ReactInternalTypes';
11 import type {Transition} from './ReactFiberTracingMarkerComponent.new';
12 +import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates.new';
13
14 // TODO: Ideally these types would be opaque but that doesn't work well with
15 // our reconciler fork infra, since these leak into non-reconciler packages.
@@ -648,6 +649,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
649 const entanglements = root.entanglements;
650 const eventTimes = root.eventTimes;
651 const expirationTimes = root.expirationTimes;
652 + const hiddenUpdates = root.hiddenUpdates;
653
654 // Clear the lanes that no longer have pending work
655 let lanes = noLongerPendingLanes;
@@ -659,6 +661,21 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
661 eventTimes[index] = NoTimestamp;
662 expirationTimes[index] = NoTimestamp;
663
664 + const hiddenUpdatesForLane = hiddenUpdates[index];
665 + if (hiddenUpdatesForLane !== null) {
666 + hiddenUpdates[index] = null;
667 + // "Hidden" updates are updates that were made to a hidden component. They
668 + // have special logic associated with them because they may be entangled
669 + // with updates that occur outside that tree. But once the outer tree
670 + // commits, they behave like regular updates.
671 + for (let i = 0; i < hiddenUpdatesForLane.length; i++) {
672 + const update = hiddenUpdatesForLane[i];
673 + if (update !== null) {
674 + update.lane &= ~OffscreenLane;
675 + }
676 + }
677 + }
678 +
679 lanes &= ~lane;
680 }
681 }
@@ -694,6 +711,22 @@ export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) {
711 }
712 }
713
714 +export function markHiddenUpdate(
715 + root: FiberRoot,
716 + update: ConcurrentUpdate,
717 + lane: Lane,
718 +) {
719 + const index = laneToIndex(lane);
720 + const hiddenUpdates = root.hiddenUpdates;
721 + const hiddenUpdatesForLane = hiddenUpdates[index];
722 + if (hiddenUpdatesForLane === null) {
723 + hiddenUpdates[index] = [update];
724 + } else {
725 + hiddenUpdatesForLane.push(update);
726 + }
727 + update.lane = lane | OffscreenLane;
728 +}
729 +
730 export function getBumpedLaneForHydration(
731 root: FiberRoot,
732 renderLanes: Lanes,
packages/react-reconciler/src/ReactFiberOffscreenComponent.js
+3 -1
@@ -38,4 +38,6 @@ export type OffscreenQueue = {|
38 transitions: Array<Transition> | null,
39 |} | null;
40
41 -export type OffscreenInstance = {};
41 +export type OffscreenInstance = {|
42 + isHidden: boolean,
43 +|};
packages/react-reconciler/src/ReactFiberRoot.new.js
+2
@@ -80,6 +80,8 @@ function FiberRootNode(
80 this.entangledLanes = NoLanes;
81 this.entanglements = createLaneMap(NoLanes);
82
83 + this.hiddenUpdates = createLaneMap(null);
84 +
85 this.identifierPrefix = identifierPrefix;
86 this.onRecoverableError = onRecoverableError;
87
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+6 -1
@@ -199,6 +199,7 @@ import {
199 import {
200 enqueueConcurrentRenderForLane,
201 finishQueueingConcurrentUpdates,
202 + getConcurrentlyUpdatedLanes,
203 } from './ReactFiberConcurrentUpdates.new';
204
205 import {
@@ -425,6 +426,10 @@ export function getWorkInProgressRoot(): FiberRoot | null {
426 return workInProgressRoot;
427 }
428
429 +export function getWorkInProgressRootRenderLanes(): Lanes {
430 + return workInProgressRootRenderLanes;
431 +}
432 +
433 export function requestEventTime() {
434 if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
435 // We're inside React, so it's fine to read the actual time.
@@ -2059,7 +2064,7 @@ function commitRootImpl(
2064
2065 // Make sure to account for lanes that were updated by a concurrent event
2066 // during the render phase; don't mark them as finished.
2062 - const concurrentlyUpdatedLanes = finishQueueingConcurrentUpdates();
2067 + const concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes();
2068 remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes);
2069
2070 markRootFinished(root, remainingLanes);
packages/react-reconciler/src/ReactInternalTypes.js
+2
@@ -27,6 +27,7 @@ import type {RootTag} from './ReactRootTags';
27 import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig';
28 import type {Cache} from './ReactFiberCacheComponent.old';
29 import type {Transition} from './ReactFiberTracingMarkerComponent.new';
30 +import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates.new';
31
32 // Unwind Circular: moved from ReactFiberHooks.old
33 export type HookType =
@@ -225,6 +226,7 @@ type BaseFiberRootProperties = {|
226 callbackPriority: Lane,
227 eventTimes: LaneMap<number>,
228 expirationTimes: LaneMap<number>,
229 + hiddenUpdates: LaneMap<Array<ConcurrentUpdate> | null>,
230
231 pendingLanes: Lanes,
232 suspendedLanes: Lanes,
packages/react-reconciler/src/__tests__/ReactOffscreen-test.js
+122
@@ -6,6 +6,7 @@ let LegacyHidden;
6 let Offscreen;
7 let useState;
8 let useLayoutEffect;
9 +let useEffect;
10
11 describe('ReactOffscreen', () => {
12 beforeEach(() => {
@@ -19,6 +20,7 @@ describe('ReactOffscreen', () => {
20 Offscreen = React.unstable_Offscreen;
21 useState = React.useState;
22 useLayoutEffect = React.useLayoutEffect;
23 + useEffect = React.useEffect;
24 });
25
26 function Text(props) {
@@ -473,4 +475,124 @@ describe('ReactOffscreen', () => {
475 // Now it's visible
476 expect(root).toMatchRenderedOutput(<span>Hi</span>);
477 });
478 +
479 + // Only works in new reconciler
480 + // @gate variant
481 + it('revealing a hidden tree at high priority does not cause tearing', async () => {
482 + // When revealing an offscreen tree, we need to include updates that were
483 + // previously deferred because the tree was hidden, even if they are lower
484 + // priority than the current render. However, we should *not* include low
485 + // priority updates that are entangled with updates outside of the hidden
486 + // tree, because that can cause tearing.
487 + //
488 + // This test covers a scenario where an update multiple updates inside a
489 + // hidden tree share the same lane, but are processed at different times
490 + // because of the timing of when they were scheduled.
491 +
492 + let setInner;
493 + function Child({outer}) {
494 + const [inner, _setInner] = useState(0);
495 + setInner = _setInner;
496 +
497 + useEffect(() => {
498 + // Inner and outer values are always updated simultaneously, so they
499 + // should always be consistent.
500 + if (inner !== outer) {
501 + Scheduler.unstable_yieldValue(
502 + 'Tearing! Inner and outer are inconsistent!',
503 + );
504 + } else {
505 + Scheduler.unstable_yieldValue('Inner and outer are consistent');
506 + }
507 + }, [inner, outer]);
508 +
509 + return <Text text={'Inner: ' + inner} />;
510 + }
511 +
512 + let setOuter;
513 + function App({show}) {
514 + const [outer, _setOuter] = useState(0);
515 + setOuter = _setOuter;
516 + return (
517 + <>
518 + <Text text={'Outer: ' + outer} />
519 + <Offscreen mode={show ? 'visible' : 'hidden'}>
520 + <Child outer={outer} />
521 + </Offscreen>
522 + </>
523 + );
524 + }
525 +
526 + // Render a hidden tree
527 + const root = ReactNoop.createRoot();
528 + await act(async () => {
529 + root.render(<App show={false} />);
530 + });
531 + expect(Scheduler).toHaveYielded([
532 + 'Outer: 0',
533 + 'Inner: 0',
534 + 'Inner and outer are consistent',
535 + ]);
536 + expect(root).toMatchRenderedOutput(
537 + <>
538 + <span prop="Outer: 0" />
539 + <span hidden={true} prop="Inner: 0" />
540 + </>,
541 + );
542 +
543 + await act(async () => {
544 + // Update a value both inside and outside the hidden tree. These values
545 + // must always be consistent.
546 + setOuter(1);
547 + setInner(1);
548 + // Only the outer updates finishes because the inner update is inside a
549 + // hidden tree. The outer update is deferred to a later render.
550 + expect(Scheduler).toFlushUntilNextPaint(['Outer: 1']);
551 + expect(root).toMatchRenderedOutput(
552 + <>
553 + <span prop="Outer: 1" />
554 + <span hidden={true} prop="Inner: 0" />
555 + </>,
556 + );
557 +
558 + // Before the inner update can finish, we receive another pair of updates.
559 + setOuter(2);
560 + setInner(2);
561 +
562 + // Also, before either of these new updates are processed, the hidden
563 + // tree is revealed at high priority.
564 + ReactNoop.flushSync(() => {
565 + root.render(<App show={true} />);
566 + });
567 +
568 + expect(Scheduler).toHaveYielded([
569 + 'Outer: 1',
570 +
571 + // There are two pending updates on Inner, but only the first one
572 + // is processed, even though they share the same lane. If the second
573 + // update were erroneously processed, then Inner would be inconsistent
574 + // with Outer.
575 + 'Inner: 1',
576 +
577 + 'Inner and outer are consistent',
578 + ]);
579 + expect(root).toMatchRenderedOutput(
580 + <>
581 + <span prop="Outer: 1" />
582 + <span prop="Inner: 1" />
583 + </>,
584 + );
585 + });
586 + expect(Scheduler).toHaveYielded([
587 + 'Outer: 2',
588 + 'Inner: 2',
589 + 'Inner and outer are consistent',
590 + ]);
591 + expect(root).toMatchRenderedOutput(
592 + <>
593 + <span prop="Outer: 2" />
594 + <span prop="Inner: 2" />
595 + </>,
596 + );
597 + });
598 });
packages/react-reconciler/src/__tests__/ReactOffscreenSuspense-test.js new
+246
@@ -0,0 +1,246 @@
1 +let React;
2 +let ReactNoop;
3 +let Scheduler;
4 +let act;
5 +let Offscreen;
6 +let Suspense;
7 +let useState;
8 +let useEffect;
9 +let textCache;
10 +
11 +describe('ReactOffscreen', () => {
12 + beforeEach(() => {
13 + jest.resetModules();
14 +
15 + React = require('react');
16 + ReactNoop = require('react-noop-renderer');
17 + Scheduler = require('scheduler');
18 + act = require('jest-react').act;
19 + Offscreen = React.unstable_Offscreen;
20 + Suspense = React.Suspense;
21 + useState = React.useState;
22 + useEffect = React.useEffect;
23 +
24 + textCache = new Map();
25 + });
26 +
27 + function resolveText(text) {
28 + const record = textCache.get(text);
29 + if (record === undefined) {
30 + const newRecord = {
31 + status: 'resolved',
32 + value: text,
33 + };
34 + textCache.set(text, newRecord);
35 + } else if (record.status === 'pending') {
36 + const thenable = record.value;
37 + record.status = 'resolved';
38 + record.value = text;
39 + thenable.pings.forEach(t => t());
40 + }
41 + }
42 +
43 + function readText(text) {
44 + const record = textCache.get(text);
45 + if (record !== undefined) {
46 + switch (record.status) {
47 + case 'pending':
48 + Scheduler.unstable_yieldValue(`Suspend! [${text}]`);
49 + throw record.value;
50 + case 'rejected':
51 + throw record.value;
52 + case 'resolved':
53 + return record.value;
54 + }
55 + } else {
56 + Scheduler.unstable_yieldValue(`Suspend! [${text}]`);
57 + const thenable = {
58 + pings: [],
59 + then(resolve) {
60 + if (newRecord.status === 'pending') {
61 + thenable.pings.push(resolve);
62 + } else {
63 + Promise.resolve().then(() => resolve(newRecord.value));
64 + }
65 + },
66 + };
67 +
68 + const newRecord = {
69 + status: 'pending',
70 + value: thenable,
71 + };
72 + textCache.set(text, newRecord);
73 +
74 + throw thenable;
75 + }
76 + }
77 +
78 + function Text({text}) {
79 + Scheduler.unstable_yieldValue(text);
80 + return text;
81 + }
82 +
83 + function AsyncText({text}) {
84 + readText(text);
85 + Scheduler.unstable_yieldValue(text);
86 + return text;
87 + }
88 +
89 + // Only works in new reconciler
90 + // @gate variant
91 + test('detect updates to a hidden tree during a concurrent event', async () => {
92 + // This is a pretty complex test case. It relates to how we detect if an
93 + // update is made to a hidden tree: when scheduling the update, we walk up
94 + // the fiber return path to see if any of the parents is a hidden Offscreen
95 + // component. This doesn't work if there's already a render in progress,
96 + // because the tree might be about to flip to hidden. To avoid a data race,
97 + // queue updates atomically: wait to queue the update until after the
98 + // current render has finished.
99 +
100 + let setInner;
101 + function Child({outer}) {
102 + const [inner, _setInner] = useState(0);
103 + setInner = _setInner;
104 +
105 + useEffect(() => {
106 + // Inner and outer values are always updated simultaneously, so they
107 + // should always be consistent.
108 + if (inner !== outer) {
109 + Scheduler.unstable_yieldValue(
110 + 'Tearing! Inner and outer are inconsistent!',
111 + );
112 + } else {
113 + Scheduler.unstable_yieldValue('Inner and outer are consistent');
114 + }
115 + }, [inner, outer]);
116 +
117 + return <Text text={'Inner: ' + inner} />;
118 + }
119 +
120 + let setOuter;
121 + function App({show}) {
122 + const [outer, _setOuter] = useState(0);
123 + setOuter = _setOuter;
124 + return (
125 + <>
126 + <span>
127 + <Text text={'Outer: ' + outer} />
128 + </span>
129 + <Offscreen mode={show ? 'visible' : 'hidden'}>
130 + <span>
131 + <Child outer={outer} />
132 + </span>
133 + </Offscreen>
134 + <Suspense fallback={<Text text="Loading..." />}>
135 + <span>
136 + <AsyncText text={'Async: ' + outer} />
137 + </span>
138 + </Suspense>
139 + </>
140 + );
141 + }
142 +
143 + // Render a hidden tree
144 + const root = ReactNoop.createRoot();
145 + resolveText('Async: 0');
146 + await act(async () => {
147 + root.render(<App show={true} />);
148 + });
149 + expect(Scheduler).toHaveYielded([
150 + 'Outer: 0',
151 + 'Inner: 0',
152 + 'Async: 0',
153 + 'Inner and outer are consistent',
154 + ]);
155 + expect(root).toMatchRenderedOutput(
156 + <>
157 + <span>Outer: 0</span>
158 + <span>Inner: 0</span>
159 + <span>Async: 0</span>
160 + </>,
161 + );
162 +
163 + await act(async () => {
164 + // Update a value both inside and outside the hidden tree. These values
165 + // must always be consistent.
166 + setOuter(1);
167 + setInner(1);
168 + // In the same render, also hide the offscreen tree.
169 + root.render(<App show={false} />);
170 +
171 + expect(Scheduler).toFlushAndYieldThrough([
172 + // The outer update will commit, but the inner update is deferred until
173 + // a later render.
174 + 'Outer: 1',
175 +
176 + // Something suspended. This means we won't commit immediately; there
177 + // will be an async gap between render and commit. In this test, we will
178 + // use this property to schedule a concurrent update. The fact that
179 + // we're using Suspense to schedule a concurrent update is not directly
180 + // relevant to the test — we could also use time slicing, but I've
181 + // chosen to use Suspense the because implementation details of time
182 + // slicing are more volatile.
183 + 'Suspend! [Async: 1]',
184 +
185 + 'Loading...',
186 + ]);
187 + // Assert that we haven't committed quite yet
188 + expect(root).toMatchRenderedOutput(
189 + <>
190 + <span>Outer: 0</span>
191 + <span>Inner: 0</span>
192 + <span>Async: 0</span>
193 + </>,
194 + );
195 +
196 + // Before the tree commits, schedule a concurrent event. The inner update
197 + // is to a tree that's just about to be hidden.
198 + setOuter(2);
199 + setInner(2);
200 +
201 + // Commit the previous render.
202 + jest.runAllTimers();
203 + expect(root).toMatchRenderedOutput(
204 + <>
205 + <span>Outer: 1</span>
206 + <span hidden={true}>Inner: 0</span>
207 + <span hidden={true}>Async: 0</span>
208 + Loading...
209 + </>,
210 + );
211 +
212 + // Now reveal the hidden tree at high priority.
213 + ReactNoop.flushSync(() => {
214 + root.render(<App show={true} />);
215 + });
216 + expect(Scheduler).toHaveYielded([
217 + 'Outer: 1',
218 +
219 + // There are two pending updates on Inner, but only the first one
220 + // is processed, even though they share the same lane. If the second
221 + // update were erroneously processed, then Inner would be inconsistent
222 + // with Outer.
223 + 'Inner: 1',
224 +
225 + 'Suspend! [Async: 1]',
226 + 'Loading...',
227 + 'Inner and outer are consistent',
228 + ]);
229 + });
230 + expect(Scheduler).toHaveYielded([
231 + 'Outer: 2',
232 + 'Inner: 2',
233 + 'Suspend! [Async: 2]',
234 + 'Loading...',
235 + 'Inner and outer are consistent',
236 + ]);
237 + expect(root).toMatchRenderedOutput(
238 + <>
239 + <span>Outer: 2</span>
240 + <span>Inner: 2</span>
241 + <span hidden={true}>Async: 0</span>
242 + Loading...
243 + </>,
244 + );
245 + });
246 +});
scripts/merge-fork/forked-revisions
+1
@@ -1 +1,2 @@
1 +31882b5dd66f34f70d341ea2781cacbe802bf4d5 [FORKED] Bugfix: Revealing a hidden update
2 17691acc071d56261d43c3cf183f287d983baa9b [FORKED] Don't update childLanes until after current render