From 8b580a89d6dbbde8a3ed69475899addef1751116 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 23 Sep 2019 20:52:48 -0700 Subject: [PATCH] Idle updates should not be blocked by hidden work (#16871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- .../src/ReactFiberExpirationTime.js | 13 ++++- .../src/ReactFiberWorkLoop.js | 29 +++++----- ...ReactSchedulerIntegration-test.internal.js | 56 +++++++++++++++++++ .../ReactDOMTracing-test.internal.js | 14 ++++- .../jest/matchers/schedulerTestMatchers.js | 10 ++++ 5 files changed, 104 insertions(+), 18 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberExpirationTime.js b/packages/react-reconciler/src/ReactFiberExpirationTime.js index e61ed56079e1..a5a9fd12f903 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -21,9 +21,16 @@ import { export type ExpirationTime = number; export const NoWork = 0; -// TODO: Think of a better name for Never. +// TODO: Think of a better name for Never. The key difference with Idle 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. So one possible +// name is Offscreen. However, it also includes 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. export const Never = 1; -// TODO: Use the Idle expiration time for idle state updates +// Idle is slightly higher priority than Never. It must completely finish in +// order to be consistent. export const Idle = 2; export const Sync = MAX_SIGNED_31_BIT_INT; export const Batched = Sync - 1; @@ -115,7 +122,7 @@ export function inferPriorityFromExpirationTime( if (expirationTime === Sync) { return ImmediatePriority; } - if (expirationTime === Never) { + if (expirationTime === Never || expirationTime === Idle) { return IdlePriority; } const msUntil = diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 50b3d2aaefac..389ff45ff323 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -321,6 +321,7 @@ export function computeExpirationForFiber( if ((executionContext & RenderContext) !== NoContext) { // Use whatever time we're already rendering + // TODO: Should there be a way to opt out, like with `runWithPriority`? return renderExpirationTime; } @@ -347,7 +348,7 @@ export function computeExpirationForFiber( expirationTime = computeAsyncExpiration(currentTime); break; case IdlePriority: - expirationTime = Never; + expirationTime = Idle; break; default: invariant(false, 'Expected a valid priority level'); @@ -1406,14 +1407,14 @@ export function markRenderEventTimeAndConfig( ): void { if ( expirationTime < workInProgressRootLatestProcessedExpirationTime && - expirationTime > Never + expirationTime > Idle ) { workInProgressRootLatestProcessedExpirationTime = expirationTime; } if (suspenseConfig !== null) { if ( expirationTime < workInProgressRootLatestSuspenseTimeout && - expirationTime > Never + expirationTime > Idle ) { workInProgressRootLatestSuspenseTimeout = expirationTime; // Most of the time we only have one config and getting wrong is not bad. @@ -2203,24 +2204,25 @@ function commitLayoutEffects( } export function flushPassiveEffects() { + if (pendingPassiveEffectsRenderPriority !== NoPriority) { + const priorityLevel = + pendingPassiveEffectsRenderPriority > NormalPriority + ? NormalPriority + : pendingPassiveEffectsRenderPriority; + pendingPassiveEffectsRenderPriority = NoPriority; + return runWithPriority(priorityLevel, flushPassiveEffectsImpl); + } +} + +function flushPassiveEffectsImpl() { if (rootWithPendingPassiveEffects === null) { return false; } const root = rootWithPendingPassiveEffects; const expirationTime = pendingPassiveEffectsExpirationTime; - const renderPriorityLevel = pendingPassiveEffectsRenderPriority; rootWithPendingPassiveEffects = null; pendingPassiveEffectsExpirationTime = NoWork; - pendingPassiveEffectsRenderPriority = NoPriority; - const priorityLevel = - renderPriorityLevel > NormalPriority ? NormalPriority : renderPriorityLevel; - return runWithPriority( - priorityLevel, - flushPassiveEffectsImpl.bind(null, root, expirationTime), - ); -} -function flushPassiveEffectsImpl(root, expirationTime) { invariant( (executionContext & (RenderContext | CommitContext)) === NoContext, 'Cannot flush passive effects while already rendering.', @@ -2263,6 +2265,7 @@ function flushPassiveEffectsImpl(root, expirationTime) { } executionContext = prevExecutionContext; + flushSyncCallbackQueue(); // If additional passive effects were scheduled, increment a counter. If this diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js index 6f91e8323b2f..8197dbc0fa2b 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js @@ -352,4 +352,60 @@ describe('ReactSchedulerIntegration', () => { Scheduler.unstable_flushUntilNextPaint(); expect(Scheduler).toHaveYielded(['A', 'B', 'C']); }); + + it('idle updates are not blocked by offscreen work', async () => { + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return text; + } + + function App({label}) { + return ( + <> + + + + ); + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + + // Commit the visible content + expect(Scheduler).toFlushUntilNextPaint(['Visible: A']); + expect(root).toMatchRenderedOutput( + <> + Visible: A +