Skip to content

Commit 8b580a8

Browse files
authored
Idle updates should not be blocked by hidden work (#16871)
* Idle updates should not be blocked by hidden work Use the special `Idle` expiration time for updates that are triggered at Scheduler's `IdlePriority`, instead of `Never`. The key difference between Idle and Never¹ is that Never work can be committed in an inconsistent state without tearing the UI. The main example is offscreen content, like a hidden subtree. ¹ "Never" isn't the best name. I originally called it that because it "never" expires, but neither does Idle. Since it's mostly used for offscreen subtrees, we could call it "Offscreen." However, it's also used for dehydrated Suspense boundaries, which are inconsistent in the sense that they haven't finished yet, but aren't visibly inconsistent because the server rendered HTML matches what the hydrated tree would look like. * Reset as early as possible using local variable * Updates in a hidden effect should be Idle I had made them Never to avoid an extra render when a hidden effect updates the hidden component -- if they are Idle, we have to render once at Idle, which bails out on the hidden subtree, then again at Never to actually process the update -- but the problem of needing an extra render pass to bail out hidden updates already exists and we should fix that properly instead of adding yet another special case.
1 parent c5e7190 commit 8b580a8

File tree

5 files changed

+104
-18
lines changed

5 files changed

+104
-18
lines changed

packages/react-reconciler/src/ReactFiberExpirationTime.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,16 @@ import {
2121
export type ExpirationTime = number;
2222

2323
export const NoWork = 0;
24-
// TODO: Think of a better name for Never.
24+
// TODO: Think of a better name for Never. The key difference with Idle is that
25+
// Never work can be committed in an inconsistent state without tearing the UI.
26+
// The main example is offscreen content, like a hidden subtree. So one possible
27+
// name is Offscreen. However, it also includes dehydrated Suspense boundaries,
28+
// which are inconsistent in the sense that they haven't finished yet, but
29+
// aren't visibly inconsistent because the server rendered HTML matches what the
30+
// hydrated tree would look like.
2531
export const Never = 1;
26-
// TODO: Use the Idle expiration time for idle state updates
32+
// Idle is slightly higher priority than Never. It must completely finish in
33+
// order to be consistent.
2734
export const Idle = 2;
2835
export const Sync = MAX_SIGNED_31_BIT_INT;
2936
export const Batched = Sync - 1;
@@ -115,7 +122,7 @@ export function inferPriorityFromExpirationTime(
115122
if (expirationTime === Sync) {
116123
return ImmediatePriority;
117124
}
118-
if (expirationTime === Never) {
125+
if (expirationTime === Never || expirationTime === Idle) {
119126
return IdlePriority;
120127
}
121128
const msUntil =

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ export function computeExpirationForFiber(
321321

322322
if ((executionContext & RenderContext) !== NoContext) {
323323
// Use whatever time we're already rendering
324+
// TODO: Should there be a way to opt out, like with `runWithPriority`?
324325
return renderExpirationTime;
325326
}
326327

@@ -347,7 +348,7 @@ export function computeExpirationForFiber(
347348
expirationTime = computeAsyncExpiration(currentTime);
348349
break;
349350
case IdlePriority:
350-
expirationTime = Never;
351+
expirationTime = Idle;
351352
break;
352353
default:
353354
invariant(false, 'Expected a valid priority level');
@@ -1406,14 +1407,14 @@ export function markRenderEventTimeAndConfig(
14061407
): void {
14071408
if (
14081409
expirationTime < workInProgressRootLatestProcessedExpirationTime &&
1409-
expirationTime > Never
1410+
expirationTime > Idle
14101411
) {
14111412
workInProgressRootLatestProcessedExpirationTime = expirationTime;
14121413
}
14131414
if (suspenseConfig !== null) {
14141415
if (
14151416
expirationTime < workInProgressRootLatestSuspenseTimeout &&
1416-
expirationTime > Never
1417+
expirationTime > Idle
14171418
) {
14181419
workInProgressRootLatestSuspenseTimeout = expirationTime;
14191420
// Most of the time we only have one config and getting wrong is not bad.
@@ -2203,24 +2204,25 @@ function commitLayoutEffects(
22032204
}
22042205

22052206
export function flushPassiveEffects() {
2207+
if (pendingPassiveEffectsRenderPriority !== NoPriority) {
2208+
const priorityLevel =
2209+
pendingPassiveEffectsRenderPriority > NormalPriority
2210+
? NormalPriority
2211+
: pendingPassiveEffectsRenderPriority;
2212+
pendingPassiveEffectsRenderPriority = NoPriority;
2213+
return runWithPriority(priorityLevel, flushPassiveEffectsImpl);
2214+
}
2215+
}
2216+
2217+
function flushPassiveEffectsImpl() {
22062218
if (rootWithPendingPassiveEffects === null) {
22072219
return false;
22082220
}
22092221
const root = rootWithPendingPassiveEffects;
22102222
const expirationTime = pendingPassiveEffectsExpirationTime;
2211-
const renderPriorityLevel = pendingPassiveEffectsRenderPriority;
22122223
rootWithPendingPassiveEffects = null;
22132224
pendingPassiveEffectsExpirationTime = NoWork;
2214-
pendingPassiveEffectsRenderPriority = NoPriority;
2215-
const priorityLevel =
2216-
renderPriorityLevel > NormalPriority ? NormalPriority : renderPriorityLevel;
2217-
return runWithPriority(
2218-
priorityLevel,
2219-
flushPassiveEffectsImpl.bind(null, root, expirationTime),
2220-
);
2221-
}
22222225

2223-
function flushPassiveEffectsImpl(root, expirationTime) {
22242226
invariant(
22252227
(executionContext & (RenderContext | CommitContext)) === NoContext,
22262228
'Cannot flush passive effects while already rendering.',
@@ -2263,6 +2265,7 @@ function flushPassiveEffectsImpl(root, expirationTime) {
22632265
}
22642266

22652267
executionContext = prevExecutionContext;
2268+
22662269
flushSyncCallbackQueue();
22672270

22682271
// If additional passive effects were scheduled, increment a counter. If this

packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,4 +352,60 @@ describe('ReactSchedulerIntegration', () => {
352352
Scheduler.unstable_flushUntilNextPaint();
353353
expect(Scheduler).toHaveYielded(['A', 'B', 'C']);
354354
});
355+
356+
it('idle updates are not blocked by offscreen work', async () => {
357+
function Text({text}) {
358+
Scheduler.unstable_yieldValue(text);
359+
return text;
360+
}
361+
362+
function App({label}) {
363+
return (
364+
<>
365+
<Text text={`Visible: ` + label} />
366+
<div hidden={true}>
367+
<Text text={`Hidden: ` + label} />
368+
</div>
369+
</>
370+
);
371+
}
372+
373+
const root = ReactNoop.createRoot();
374+
await ReactNoop.act(async () => {
375+
root.render(<App label="A" />);
376+
377+
// Commit the visible content
378+
expect(Scheduler).toFlushUntilNextPaint(['Visible: A']);
379+
expect(root).toMatchRenderedOutput(
380+
<>
381+
Visible: A
382+
<div hidden={true} />
383+
</>,
384+
);
385+
386+
// Before the hidden content has a chance to render, schedule an
387+
// idle update
388+
runWithPriority(IdlePriority, () => {
389+
root.render(<App label="B" />);
390+
});
391+
392+
// The next commit should only include the visible content
393+
expect(Scheduler).toFlushUntilNextPaint(['Visible: B']);
394+
expect(root).toMatchRenderedOutput(
395+
<>
396+
Visible: B
397+
<div hidden={true} />
398+
</>,
399+
);
400+
});
401+
402+
// The hidden content commits later
403+
expect(Scheduler).toHaveYielded(['Hidden: B']);
404+
expect(root).toMatchRenderedOutput(
405+
<>
406+
Visible: B
407+
<div hidden={true}>Hidden: B</div>
408+
</>,
409+
);
410+
});
355411
});

packages/react/src/__tests__/ReactDOMTracing-test.internal.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,12 @@ describe('ReactDOMTracing', () => {
134134
expect(
135135
onInteractionScheduledWorkCompleted,
136136
).toHaveBeenLastNotifiedOfInteraction(interaction);
137-
expect(onRender).toHaveBeenCalledTimes(3);
137+
// TODO: This is 4 instead of 3 because this update was scheduled at
138+
// idle priority, and idle updates are slightly higher priority than
139+
// offscreen work. So it takes two render passes to finish it. Profiler
140+
// calls `onRender` for the first render even though everything
141+
// bails out.
142+
expect(onRender).toHaveBeenCalledTimes(4);
138143
expect(onRender).toHaveLastRenderedWithInteractions(
139144
new Set([interaction]),
140145
);
@@ -281,7 +286,12 @@ describe('ReactDOMTracing', () => {
281286
expect(
282287
onInteractionScheduledWorkCompleted,
283288
).toHaveBeenLastNotifiedOfInteraction(interaction);
284-
expect(onRender).toHaveBeenCalledTimes(3);
289+
// TODO: This is 4 instead of 3 because this update was scheduled at
290+
// idle priority, and idle updates are slightly higher priority than
291+
// offscreen work. So it takes two render passes to finish it. Profiler
292+
// calls `onRender` for the first render even though everything
293+
// bails out.
294+
expect(onRender).toHaveBeenCalledTimes(4);
285295
expect(onRender).toHaveLastRenderedWithInteractions(
286296
new Set([interaction]),
287297
);

scripts/jest/matchers/schedulerTestMatchers.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ function toFlushAndYieldThrough(Scheduler, expectedYields) {
4444
});
4545
}
4646

47+
function toFlushUntilNextPaint(Scheduler, expectedYields) {
48+
assertYieldsWereCleared(Scheduler);
49+
Scheduler.unstable_flushUntilNextPaint();
50+
const actualYields = Scheduler.unstable_clearYields();
51+
return captureAssertion(() => {
52+
expect(actualYields).toEqual(expectedYields);
53+
});
54+
}
55+
4756
function toFlushWithoutYielding(Scheduler) {
4857
return toFlushAndYield(Scheduler, []);
4958
}
@@ -76,6 +85,7 @@ function toFlushAndThrow(Scheduler, ...rest) {
7685
module.exports = {
7786
toFlushAndYield,
7887
toFlushAndYieldThrough,
88+
toFlushUntilNextPaint,
7989
toFlushWithoutYielding,
8090
toFlushExpired,
8191
toHaveYielded,

0 commit comments

Comments
 (0)