From d5e8f79cf4d11fa7eee263b3f937deecbe65ffd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 12 Dec 2024 23:06:07 -0500 Subject: [PATCH] [Fiber] Use hydration lanes when scheduling hydration work (#31751) When scheduling the initial root and when using `unstable_scheduleHydration` we should use the Hydration Lanes rather than the raw update lane. This ensures that we're always hydrating using a Hydration Lane or the Offscreen Lane rather than other lanes getting some random hydration in it. This fixes an issue where updating a root while it is still hydrating causes it to trigger client rendering when it could just hydrate and then apply the update on top of that. It also fixes a potential performance issue where `unstable_scheduleHydration` gets batched with an update that then ends up forcing an update of a boundary that requires it to rewind to do the hydration lane anyway. Might as well just start with the hydration without the update applied first. I added a kill switch (`enableHydrationLaneScheduling`) just in case but seems very safe given that using `unstable_scheduleHydration` at all is very rare and updating the root before the shell hydrates is extremely rare (and used to trigger a recoverable error). --- .../ReactDOMFizzShellHydration-test.js | 33 ++++++ .../react-reconciler/src/ReactFiberLane.js | 101 +++++++++--------- .../src/ReactFiberReconciler.js | 16 ++- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 2 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 2 + .../shared/forks/ReactFeatureFlags.www.js | 2 + 10 files changed, 110 insertions(+), 51 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js index 5cafd83fdd243..d21059ba6cf0b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js @@ -255,6 +255,39 @@ describe('ReactDOMFizzShellHydration', () => { }, ); + // @gate enableHydrationLaneScheduling + it( + 'updating the root at same priority as initial hydration does not ' + + 'force a client render', + async () => { + function App() { + return ; + } + + // Server render + await resolveText('Initial'); + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + assertLog(['Initial']); + + await clientAct(async () => { + let root; + startTransition(() => { + root = ReactDOMClient.hydrateRoot(container, ); + }); + // This has lower priority than the initial hydration, so the update + // won't be processed until after hydration finishes. + startTransition(() => { + root.render(); + }); + }); + assertLog(['Initial', 'Updated']); + expect(container.textContent).toBe('Updated'); + }, + ); + it('updating the root while the shell is suspended forces a client render', async () => { function App() { return ; diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index b98019046e3c2..ee3a0a3df2b60 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -995,61 +995,66 @@ export function getBumpedLaneForHydration( renderLanes: Lanes, ): Lane { const renderLane = getHighestPriorityLane(renderLanes); - - let lane; - if ((renderLane & SyncUpdateLanes) !== NoLane) { - lane = SyncHydrationLane; - } else { - switch (renderLane) { - case SyncLane: - lane = SyncHydrationLane; - break; - case InputContinuousLane: - lane = InputContinuousHydrationLane; - break; - case DefaultLane: - lane = DefaultHydrationLane; - break; - case TransitionLane1: - case TransitionLane2: - case TransitionLane3: - case TransitionLane4: - case TransitionLane5: - case TransitionLane6: - case TransitionLane7: - case TransitionLane8: - case TransitionLane9: - case TransitionLane10: - case TransitionLane11: - case TransitionLane12: - case TransitionLane13: - case TransitionLane14: - case TransitionLane15: - case RetryLane1: - case RetryLane2: - case RetryLane3: - case RetryLane4: - lane = TransitionHydrationLane; - break; - case IdleLane: - lane = IdleHydrationLane; - break; - default: - // Everything else is already either a hydration lane, or shouldn't - // be retried at a hydration lane. - lane = NoLane; - break; - } - } - + const bumpedLane = + (renderLane & SyncUpdateLanes) !== NoLane + ? // Unify sync lanes. We don't do this inside getBumpedLaneForHydrationByLane + // because that causes things to flush synchronously when they shouldn't. + // TODO: This is not coherent but that's beacuse the unification is not coherent. + // We need to get merge these into an actual single lane. + SyncHydrationLane + : getBumpedLaneForHydrationByLane(renderLane); // Check if the lane we chose is suspended. If so, that indicates that we // already attempted and failed to hydrate at that level. Also check if we're // already rendering that lane, which is rare but could happen. - if ((lane & (root.suspendedLanes | renderLanes)) !== NoLane) { + // TODO: This should move into the caller to decide whether giving up is valid. + if ((bumpedLane & (root.suspendedLanes | renderLanes)) !== NoLane) { // Give up trying to hydrate and fall back to client render. return NoLane; } + return bumpedLane; +} +export function getBumpedLaneForHydrationByLane(lane: Lane): Lane { + switch (lane) { + case SyncLane: + lane = SyncHydrationLane; + break; + case InputContinuousLane: + lane = InputContinuousHydrationLane; + break; + case DefaultLane: + lane = DefaultHydrationLane; + break; + case TransitionLane1: + case TransitionLane2: + case TransitionLane3: + case TransitionLane4: + case TransitionLane5: + case TransitionLane6: + case TransitionLane7: + case TransitionLane8: + case TransitionLane9: + case TransitionLane10: + case TransitionLane11: + case TransitionLane12: + case TransitionLane13: + case TransitionLane14: + case TransitionLane15: + case RetryLane1: + case RetryLane2: + case RetryLane3: + case RetryLane4: + lane = TransitionHydrationLane; + break; + case IdleLane: + lane = IdleHydrationLane; + break; + default: + // Everything else is already either a hydration lane, or shouldn't + // be retried at a hydration lane. + lane = NoLane; + break; + } return lane; } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 31a7fbeccf36b..98ef2ef93fc46 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -38,7 +38,10 @@ import { } from './ReactWorkTags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import isArray from 'shared/isArray'; -import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags'; +import { + enableSchedulingProfiler, + enableHydrationLaneScheduling, +} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { getPublicInstance, @@ -91,6 +94,7 @@ import { SelectiveHydrationLane, getHighestPriorityPendingLanes, higherPriorityLane, + getBumpedLaneForHydrationByLane, } from './ReactFiberLane'; import { scheduleRefresh, @@ -322,7 +326,10 @@ export function createHydrationContainer( // the update to schedule work on the root fiber (and, for legacy roots, to // enqueue the callback if one is provided). const current = root.current; - const lane = requestUpdateLane(current); + let lane = requestUpdateLane(current); + if (enableHydrationLaneScheduling) { + lane = getBumpedLaneForHydrationByLane(lane); + } const update = createUpdate(lane); update.callback = callback !== undefined && callback !== null ? callback : null; @@ -533,7 +540,10 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void { // their priority other than synchronously flush it. return; } - const lane = requestUpdateLane(fiber); + let lane = requestUpdateLane(fiber); + if (enableHydrationLaneScheduling) { + lane = getBumpedLaneForHydrationByLane(lane); + } const root = enqueueConcurrentRenderForLane(fiber, lane); if (root !== null) { scheduleUpdateOnFiber(root, fiber, lane); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index e82bce7b50cbe..6ccf1ce8dd828 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -116,6 +116,8 @@ export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = __EXPERIMENTAL__; +export const enableHydrationLaneScheduling = true; + // Enables useMemoCache hook, intended as a compilation target for // auto-memoization. export const enableUseMemoCacheHook = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 58ae9c4fffe41..850249b8f79fd 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -94,6 +94,7 @@ export const retryLaneExpirationMs = 5000; export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const useModernStrictMode = true; +export const enableHydrationLaneScheduling = true; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 16c156f41c5ac..419cee942a9cc 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -87,6 +87,8 @@ export const useModernStrictMode = true; export const enableSiblingPrerendering = true; export const enableUseResourceEffectHook = false; +export const enableHydrationLaneScheduling = true; + // Profiling Only export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 01462919600d2..c591794c20e83 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -49,6 +49,7 @@ export const enableMoveBefore = false; export const enableGetInspectorDataForInstanceInProduction = false; export const enableFabricCompleteRootInCommitPhase = false; export const enableHiddenSubtreeInsertionEffectCleanup = false; +export const enableHydrationLaneScheduling = true; export const enableRetryLaneExpiration = false; export const retryLaneExpirationMs = 5000; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 51d9b90d4f7be..7eef951628bba 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -82,6 +82,7 @@ export const useModernStrictMode = true; export const enableFabricCompleteRootInCommitPhase = false; export const enableSiblingPrerendering = true; export const enableUseResourceEffectHook = true; +export const enableHydrationLaneScheduling = true; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 272886cc58d07..2ca5c62988aa8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -97,5 +97,7 @@ export const enableSiblingPrerendering = true; export const enableUseResourceEffectHook = false; +export const enableHydrationLaneScheduling = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index da97bbc212876..40e996b571a96 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -64,6 +64,8 @@ export const disableInputAttributeSyncing = false; export const enableLegacyFBSupport = true; export const enableLazyContextPropagation = true; +export const enableHydrationLaneScheduling = true; + export const enableComponentPerformanceTrack = false; // Logs additional User Timing API marks for use with an experimental profiling tool.