From 676f0879f315130309262ff3532707029f0288bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:18:02 -0400 Subject: [PATCH 1/6] Reset currentEventTransitionLane after flushing sync work (#33159) This keeps track of the transition lane allocated for this event. I want to be able to use the current one within sync work flushing to know which lane needs its loading indicator cleared. It's also a bit weird that transition work scheduled inside sync updates in the same event aren't entangled with other transitions in that event when `flushSync` is. Therefore this moves it to reset after flushing. It should have no impact. Just splitting it out into a separate PR for an abundance of caution. The only thing this might affect would be if the React internals throws and it doesn't reset after. But really it doesn't really have to reset and they're all entangled anyway. --- packages/react-reconciler/src/ReactFiberRootScheduler.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index f7c26580bb9..4dfa1975a25 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -257,7 +257,6 @@ function processRootScheduleInMicrotask() { // preserve the scroll position of the previous page. syncTransitionLanes = currentEventTransitionLane; } - currentEventTransitionLane = NoLane; } const currentTime = now(); @@ -315,6 +314,9 @@ function processRootScheduleInMicrotask() { if (!hasPendingCommitEffects()) { flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false); } + + // Reset Event Transition Lane so that we allocate a new one next time. + currentEventTransitionLane = NoLane; } function scheduleTaskForRootDuringMicrotask( From 0cac32d60dd4482b27fe8a54dffbabceb22c6272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:20:59 -0400 Subject: [PATCH 2/6] [Fiber] Stash the entangled async action lane on currentEventTransitionLane (#33188) When we're entangled with an async action lane we use that lane instead of the currentEventTransitionLane. Conversely, if we start a new async action lane we reuse the currentEventTransitionLane. So they're basically supposed to be in sync but they're not if you resolve the async action and then schedule new stuff in the same event. Then you end up with two transitions in the same event with different lanes. By stashing it like this we fix that but it also gives us an opportunity to check just the currentEventTransitionLane to see if this event scheduled any regular Transition updates or Async Transitions. --- .../react-reconciler/src/ReactFiberRootScheduler.js | 11 ++++++++++- packages/react-reconciler/src/ReactFiberWorkLoop.js | 10 +--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 4dfa1975a25..9fc6728cf99 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -78,6 +78,7 @@ import { resetNestedUpdateFlag, syncNestedUpdateFlag, } from './ReactProfilerTimer'; +import {peekEntangledActionLane} from './ReactFiberAsyncAction'; // A linked list of all the roots with pending work. In an idiomatic app, // there's only a single root, but we do support multi root apps, hence this @@ -647,7 +648,15 @@ export function requestTransitionLane( // over. Our heuristic for that is whenever we enter a concurrent work loop. if (currentEventTransitionLane === NoLane) { // All transitions within the same event are assigned the same lane. - currentEventTransitionLane = claimNextTransitionLane(); + const actionScopeLane = peekEntangledActionLane(); + currentEventTransitionLane = + actionScopeLane !== NoLane + ? // We're inside an async action scope. Reuse the same lane. + actionScopeLane + : // We may or may not be inside an async action scope. If we are, this + // is the first update in that scope. Either way, we need to get a + // fresh transition lane. + claimNextTransitionLane(); } return currentEventTransitionLane; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 95285c936eb..c62691aa1f8 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -356,7 +356,6 @@ import { requestTransitionLane, } from './ReactFiberRootScheduler'; import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext'; -import {peekEntangledActionLane} from './ReactFiberAsyncAction'; import {logUncaughtError} from './ReactFiberErrorLogger'; import { deleteScheduledGesture, @@ -779,14 +778,7 @@ export function requestUpdateLane(fiber: Fiber): Lane { transition._updatedFibers.add(fiber); } - const actionScopeLane = peekEntangledActionLane(); - return actionScopeLane !== NoLane - ? // We're inside an async action scope. Reuse the same lane. - actionScopeLane - : // We may or may not be inside an async action scope. If we are, this - // is the first update in that scope. Either way, we need to get a - // fresh transition lane. - requestTransitionLane(transition); + return requestTransitionLane(transition); } return eventPriorityToLane(resolveUpdatePriority()); From 62d3f36ea79fc0a10b514d4bbcc4ba3f21b3206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:45:11 -0400 Subject: [PATCH 3/6] [Fiber] Trigger default transition indicator if needed (#33160) Stacked on #33159. This implements `onDefaultTransitionIndicator`. The sequence is: 1) In `markRootUpdated` we schedule Transition updates as needing `indicatorLanes` on the root. This tracks the lanes that currently need an indicator to either start or remain going until this lane commits. 2) Track mutations during any commit. We use the same hook that view transitions use here but instead of tracking it just per view transition scope, we also track a global boolean for the whole root. 3) If a sync/default commit had any mutations, then we clear the indicator lane for the `currentEventTransitionLane`. This requires that the lane is still active while we do these commits. See #33159. In other words, a sync update gets associated with the current transition and it is assumed to be rendering the loading state for that corresponding transition so we don't need a default indicator for this lane. 4) At the end of `processRootScheduleInMicrotask`, right before we're about to enter a new "event transition lane" scope, it is no longer possible to render any more loading states for the current transition lane. That's when we invoke `onDefaultTransitionIndicator` for any roots that have new indicator lanes. 5) When we commit, we remove the finished lanes from `indicatorLanes` and once that reaches zero again, then we can clean up the default indicator. This approach means that you can start multiple different transitions while an indicator is still going but it won't stop/restart each time. Instead, it'll wait until all are done before stopping. Follow ups: - [x] Default updates are currently not enough to cancel because those aren't flush in the same microtask. That's unfortunate. #33186 - [x] Handle async actions before the setState. Since these don't necessarily have a root this is tricky. #33190 - [x] Disable for `useDeferredValue`. ~Since it also goes through `markRootUpdated` and schedules a Transition lane it'll get a default indicator even though it probably shouldn't have one.~ EDIT: Turns out this just works because it doesn't go through `markRootUpdated` when work is left behind. - [x] Implement built-in DOM version by default. #33162 --- .../src/createReactNoop.js | 4 +- .../src/ReactFiberCommitWork.js | 18 + .../react-reconciler/src/ReactFiberLane.js | 13 + .../src/ReactFiberMutationTracking.js | 24 +- .../react-reconciler/src/ReactFiberRoot.js | 4 + .../src/ReactFiberRootScheduler.js | 42 +- .../src/ReactFiberWorkLoop.js | 30 ++ .../src/ReactInternalTypes.js | 3 + .../ReactDefaultTransitionIndicator-test.js | 358 ++++++++++++++++++ 9 files changed, 490 insertions(+), 6 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index dd5173cd837..6fbad9adac1 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -1142,9 +1142,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // TODO: Turn this on once tests are fixed // console.error(error); } - function onDefaultTransitionIndicator(): void | (() => void) { - // TODO: Allow this as an option. - } + function onDefaultTransitionIndicator(): void | (() => void) {} let idCounter = 0; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b7b17a080b1..03d78545d4e 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -20,6 +20,7 @@ import type { import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; import { + includesLoadingIndicatorLanes, includesOnlySuspenseyCommitEligibleLanes, includesOnlyViewTransitionEligibleLanes, } from './ReactFiberLane'; @@ -60,6 +61,7 @@ import { enableViewTransition, enableFragmentRefs, enableEagerAlternateStateNodeCleanup, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -268,13 +270,16 @@ import { } from './ReactFiberCommitViewTransitions'; import { viewTransitionMutationContext, + pushRootMutationContext, pushMutationContext, popMutationContext, + rootMutationContext, } from './ReactFiberMutationTracking'; import { trackNamedViewTransition, untrackNamedViewTransition, } from './ReactFiberDuplicateViewTransitions'; +import {markIndicatorHandled} from './ReactFiberRootScheduler'; // Used during the commit phase to track the state of the Offscreen component stack. // Allows us to avoid traversing the return path to find the nearest Offscreen ancestor. @@ -2216,6 +2221,7 @@ function commitMutationEffectsOnFiber( case HostRoot: { const prevProfilerEffectDuration = pushNestedEffectDurations(); + pushRootMutationContext(); if (supportsResources) { prepareToCommitHoistables(); @@ -2265,6 +2271,18 @@ function commitMutationEffectsOnFiber( ); } + popMutationContext(false); + + if ( + enableDefaultTransitionIndicator && + rootMutationContext && + includesLoadingIndicatorLanes(lanes) + ) { + // This root had a mutation. Mark this root as having rendered a manual + // loading state. + markIndicatorHandled(root); + } + break; } case HostPortal: { diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index fdada6b065a..bd7f3267efd 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -27,6 +27,7 @@ import { transitionLaneExpirationMs, retryLaneExpirationMs, disableLegacyMode, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import {clz32} from './clz32'; @@ -640,6 +641,10 @@ export function includesOnlySuspenseyCommitEligibleLanes( ); } +export function includesLoadingIndicatorLanes(lanes: Lanes): boolean { + return (lanes & (SyncLane | DefaultLane)) !== NoLanes; +} + export function includesBlockingLane(lanes: Lanes): boolean { const SyncDefaultLanes = InputContinuousHydrationLane | @@ -766,6 +771,10 @@ export function createLaneMap(initial: T): LaneMap { export function markRootUpdated(root: FiberRoot, updateLane: Lane) { root.pendingLanes |= updateLane; + if (enableDefaultTransitionIndicator) { + // Mark that this lane might need a loading indicator to be shown. + root.indicatorLanes |= updateLane & TransitionLanes; + } // If there are any suspended transitions, it's possible this new update // could unblock them. Clear the suspended lanes so that we can try rendering @@ -847,6 +856,10 @@ export function markRootFinished( root.pingedLanes = NoLanes; root.warmLanes = NoLanes; + if (enableDefaultTransitionIndicator) { + root.indicatorLanes &= remainingLanes; + } + root.expiredLanes &= remainingLanes; root.entangledLanes &= remainingLanes; diff --git a/packages/react-reconciler/src/ReactFiberMutationTracking.js b/packages/react-reconciler/src/ReactFiberMutationTracking.js index 164ec2c6edd..cb439bd68fb 100644 --- a/packages/react-reconciler/src/ReactFiberMutationTracking.js +++ b/packages/react-reconciler/src/ReactFiberMutationTracking.js @@ -7,10 +7,23 @@ * @flow */ -import {enableViewTransition} from 'shared/ReactFeatureFlags'; +import { + enableDefaultTransitionIndicator, + enableViewTransition, +} from 'shared/ReactFeatureFlags'; +export let rootMutationContext: boolean = false; export let viewTransitionMutationContext: boolean = false; +export function pushRootMutationContext(): void { + if (enableDefaultTransitionIndicator) { + rootMutationContext = false; + } + if (enableViewTransition) { + viewTransitionMutationContext = false; + } +} + export function pushMutationContext(): boolean { if (!enableViewTransition) { return false; @@ -22,12 +35,21 @@ export function pushMutationContext(): boolean { export function popMutationContext(prev: boolean): void { if (enableViewTransition) { + if (viewTransitionMutationContext) { + rootMutationContext = true; + } viewTransitionMutationContext = prev; } } export function trackHostMutation(): void { + // This is extremely hot function that must be inlined. Don't add more stuff. if (enableViewTransition) { viewTransitionMutationContext = true; + } else if (enableDefaultTransitionIndicator) { + // We only set this if enableViewTransition is not on. Otherwise we track + // it on the viewTransitionMutationContext and collect it when we pop + // to avoid more than a single operation in this hot path. + rootMutationContext = true; } } diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index cc2a528010e..e9d107bcb89 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -79,6 +79,9 @@ function FiberRootNode( this.pingedLanes = NoLanes; this.warmLanes = NoLanes; this.expiredLanes = NoLanes; + if (enableDefaultTransitionIndicator) { + this.indicatorLanes = NoLanes; + } this.errorRecoveryDisabledLanes = NoLanes; this.shellSuspendCounter = 0; @@ -94,6 +97,7 @@ function FiberRootNode( if (enableDefaultTransitionIndicator) { this.onDefaultTransitionIndicator = onDefaultTransitionIndicator; + this.pendingIndicator = null; } this.pooledCache = null; diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 9fc6728cf99..7daed077eee 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -20,6 +20,7 @@ import { enableComponentPerformanceTrack, enableYieldingBeforePassive, enableGestureTransition, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import { NoLane, @@ -80,6 +81,9 @@ import { } from './ReactProfilerTimer'; import {peekEntangledActionLane} from './ReactFiberAsyncAction'; +import noop from 'shared/noop'; +import reportGlobalError from 'shared/reportGlobalError'; + // A linked list of all the roots with pending work. In an idiomatic app, // there's only a single root, but we do support multi root apps, hence this // extra complexity. But this module is optimized for the single root case. @@ -316,8 +320,33 @@ function processRootScheduleInMicrotask() { flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false); } - // Reset Event Transition Lane so that we allocate a new one next time. - currentEventTransitionLane = NoLane; + if (currentEventTransitionLane !== NoLane) { + // Reset Event Transition Lane so that we allocate a new one next time. + currentEventTransitionLane = NoLane; + startDefaultTransitionIndicatorIfNeeded(); + } +} + +function startDefaultTransitionIndicatorIfNeeded() { + if (!enableDefaultTransitionIndicator) { + return; + } + // Check all the roots if there are any new indicators needed. + let root = firstScheduledRoot; + while (root !== null) { + if (root.indicatorLanes !== NoLanes && root.pendingIndicator === null) { + // We have new indicator lanes that requires a loading state. Start the + // default transition indicator. + try { + const onDefaultTransitionIndicator = root.onDefaultTransitionIndicator; + root.pendingIndicator = onDefaultTransitionIndicator() || noop; + } catch (x) { + root.pendingIndicator = noop; + reportGlobalError(x); + } + } + root = root.next; + } } function scheduleTaskForRootDuringMicrotask( @@ -664,3 +693,12 @@ export function requestTransitionLane( export function didCurrentEventScheduleTransition(): boolean { return currentEventTransitionLane !== NoLane; } + +export function markIndicatorHandled(root: FiberRoot): void { + if (enableDefaultTransitionIndicator) { + // The current transition event rendered a synchronous loading state. + // Clear it from the indicator lanes. We don't need to show a separate + // loading state for this lane. + root.indicatorLanes &= ~currentEventTransitionLane; + } +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index c62691aa1f8..cd5c1c14685 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -52,11 +52,14 @@ import { enableThrottledScheduling, enableViewTransition, enableGestureTransition, + enableDefaultTransitionIndicator, } from 'shared/ReactFeatureFlags'; import {resetOwnerStackLimit} from 'shared/ReactOwnerStackReset'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; +import reportGlobalError from 'shared/reportGlobalError'; + import { // Aliased because `act` will override and push to an internal queue scheduleCallback as Scheduler_scheduleCallback, @@ -3593,6 +3596,33 @@ function flushLayoutEffects(): void { const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; + if (enableDefaultTransitionIndicator) { + const cleanUpIndicator = root.pendingIndicator; + if (cleanUpIndicator !== null && root.indicatorLanes === NoLanes) { + // We have now committed all Transitions that needed the default indicator + // so we can now run the clean up function. We do this in the layout phase + // so it has the same semantics as if you did it with a useLayoutEffect or + // if it was reset automatically with useOptimistic. + const prevTransition = ReactSharedInternals.T; + ReactSharedInternals.T = null; + const previousPriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + const prevExecutionContext = executionContext; + executionContext |= CommitContext; + root.pendingIndicator = null; + try { + cleanUpIndicator(); + } catch (x) { + reportGlobalError(x); + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; + } + } + } + const subtreeHasLayoutEffects = (finishedWork.subtreeFlags & LayoutMask) !== NoFlags; const rootHasLayoutEffect = (finishedWork.flags & LayoutMask) !== NoFlags; diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index b364d4ec47a..25840749a1a 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -248,6 +248,7 @@ type BaseFiberRootProperties = { pingedLanes: Lanes, warmLanes: Lanes, expiredLanes: Lanes, + indicatorLanes: Lanes, // enableDefaultTransitionIndicator only errorRecoveryDisabledLanes: Lanes, shellSuspendCounter: number, @@ -280,7 +281,9 @@ type BaseFiberRootProperties = { errorInfo: {+componentStack?: ?string}, ) => void, + // enableDefaultTransitionIndicator only onDefaultTransitionIndicator: () => void | (() => void), + pendingIndicator: null | (() => void), formState: ReactFormState | null, diff --git a/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js new file mode 100644 index 00000000000..bc5c3ec6695 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js @@ -0,0 +1,358 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +'use strict'; + +let React; +let ReactNoop; +let Scheduler; +let act; +let use; +let useOptimistic; +let useState; +let useTransition; +let useDeferredValue; +let assertLog; +let waitForPaint; + +describe('ReactDefaultTransitionIndicator', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + const InternalTestUtils = require('internal-test-utils'); + act = InternalTestUtils.act; + assertLog = InternalTestUtils.assertLog; + waitForPaint = InternalTestUtils.waitForPaint; + use = React.use; + useOptimistic = React.useOptimistic; + useState = React.useState; + useTransition = React.useTransition; + useDeferredValue = React.useDeferredValue; + }); + + // @gate enableDefaultTransitionIndicator + it('triggers the default indicator while a transition is on-going', async () => { + let resolve; + const promise = new Promise(r => (resolve = r)); + function App() { + return use(promise); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render(); + }); + }); + + assertLog(['start']); + + await act(async () => { + await resolve('Hello'); + }); + + assertLog(['stop']); + + expect(root).toMatchRenderedOutput('Hello'); + }); + + // @gate enableDefaultTransitionIndicator + it('does not trigger the default indicator if there is a sync mutation', async () => { + const promiseA = Promise.resolve('Hi'); + let resolveB; + const promiseB = new Promise(r => (resolveB = r)); + let update; + function App({children}) { + const [state, setState] = useState(''); + update = setState; + return ( +
+ {state} + {children} +
+ ); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render({promiseA}); + }); + }); + + assertLog(['start', 'stop']); + + expect(root).toMatchRenderedOutput(
Hi
); + + await act(() => { + // TODO: This should not require a discrete update ideally but work for default too. + ReactNoop.discreteUpdates(() => { + update('Loading...'); + }); + React.startTransition(() => { + update(''); + root.render({promiseB}); + }); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Loading...Hi
); + + await act(async () => { + await resolveB('Hello'); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Hello
); + }); + + // @gate enableDefaultTransitionIndicator + it('does not trigger the default indicator if there is an optimistic update', async () => { + const promiseA = Promise.resolve('Hi'); + let resolveB; + const promiseB = new Promise(r => (resolveB = r)); + let update; + function App({children}) { + const [state, setOptimistic] = useOptimistic(''); + update = setOptimistic; + return ( +
+ {state} + {children} +
+ ); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render({promiseA}); + }); + }); + + assertLog(['start', 'stop']); + + expect(root).toMatchRenderedOutput(
Hi
); + + await act(() => { + React.startTransition(() => { + update('Loading...'); + root.render({promiseB}); + }); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Loading...Hi
); + + await act(async () => { + await resolveB('Hello'); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Hello
); + }); + + // @gate enableDefaultTransitionIndicator + it('does not trigger the default indicator if there is an isPending update', async () => { + const promiseA = Promise.resolve('Hi'); + let resolveB; + const promiseB = new Promise(r => (resolveB = r)); + let start; + function App({children}) { + const [isPending, startTransition] = useTransition(); + start = startTransition; + return ( +
+ {isPending ? 'Loading...' : ''} + {children} +
+ ); + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + React.startTransition(() => { + root.render({promiseA}); + }); + }); + + assertLog(['start', 'stop']); + + expect(root).toMatchRenderedOutput(
Hi
); + + await act(() => { + start(() => { + root.render({promiseB}); + }); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Loading...Hi
); + + await act(async () => { + await resolveB('Hello'); + }); + + assertLog([]); + + expect(root).toMatchRenderedOutput(
Hello
); + }); + + // @gate enableDefaultTransitionIndicator + it('triggers the default indicator while an async transition is ongoing', async () => { + let resolve; + const promise = new Promise(r => (resolve = r)); + let start; + function App() { + const [, startTransition] = useTransition(); + start = startTransition; + return 'Hi'; + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(() => { + root.render(); + }); + + assertLog([]); + + await act(() => { + // Start an async action but we haven't called setState yet + // TODO: This should ideally work with React.startTransition too but we don't know the root. + start(() => promise); + }); + + assertLog(['start']); + + await act(async () => { + await resolve('Hello'); + }); + + assertLog(['stop']); + + expect(root).toMatchRenderedOutput('Hi'); + }); + + it('should not trigger for useDeferredValue (sync)', async () => { + function Text({text}) { + Scheduler.log(text); + return text; + } + function App({value}) { + const deferredValue = useDeferredValue(value, 'Hi'); + return ; + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(async () => { + root.render(); + await waitForPaint(['Hi']); + expect(root).toMatchRenderedOutput('Hi'); + }); + + assertLog(['Hello']); + + expect(root).toMatchRenderedOutput('Hello'); + + assertLog([]); + + await act(async () => { + root.render(); + await waitForPaint(['Hello']); + expect(root).toMatchRenderedOutput('Hello'); + }); + + assertLog(['Bye']); + + expect(root).toMatchRenderedOutput('Bye'); + }); + + // @gate enableDefaultTransitionIndicator + it('should not trigger for useDeferredValue (transition)', async () => { + function Text({text}) { + Scheduler.log(text); + return text; + } + function App({value}) { + const deferredValue = useDeferredValue(value, 'Hi'); + return ; + } + + const root = ReactNoop.createRoot({ + onDefaultTransitionIndicator() { + Scheduler.log('start'); + return () => { + Scheduler.log('stop'); + }; + }, + }); + await act(async () => { + React.startTransition(() => { + root.render(); + }); + await waitForPaint(['start', 'Hi', 'stop']); + expect(root).toMatchRenderedOutput('Hi'); + }); + + assertLog(['Hello']); + + expect(root).toMatchRenderedOutput('Hello'); + }); +}); From b480865db0babfcad602a1a1909775069b5779f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 15:52:44 -0400 Subject: [PATCH 4/6] [Fiber] Always flush Default priority in the microtask if a Transition was scheduled (#33186) Stacked on #33160. The purpose of this is to avoid calling `onDefaultTransitionIndicator` when a Default priority update acts as the loading indicator, but still call it when unrelated Default updates happens nearby. When we schedule Default priority work that gets batched with other events in the same frame more or less. This helps optimize by doing less work. However, that batching means that we can't separate work from one setState from another. If we would consider all Default priority work in a frame when determining whether to show the default we might never show it in cases like when you have a recurring timer updating something. This instead flushes the Default priority work eagerly along with the sync work at the end of the event, if this event scheduled any Transition work. This is then used to determine if the default indicator needs to be shown. --- packages/react-reconciler/src/ReactFiberRootScheduler.js | 8 ++++++++ .../src/__tests__/ReactDefaultTransitionIndicator-test.js | 5 +---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 7daed077eee..0bcf1d580c6 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -26,6 +26,7 @@ import { NoLane, NoLanes, SyncLane, + DefaultLane, getHighestPriorityLane, getNextLanes, includesSyncLane, @@ -261,6 +262,13 @@ function processRootScheduleInMicrotask() { // render it synchronously anyway. We do this during a popstate event to // preserve the scroll position of the previous page. syncTransitionLanes = currentEventTransitionLane; + } else if (enableDefaultTransitionIndicator) { + // If we have a Transition scheduled by this event it might be paired + // with Default lane scheduled loading indicators. To unbatch it from + // other events later on, flush it early to determine whether it + // rendered an indicator. This ensures that setState in default priority + // event doesn't trigger onDefaultTransitionIndicator. + syncTransitionLanes = DefaultLane; } } diff --git a/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js index bc5c3ec6695..71848056034 100644 --- a/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDefaultTransitionIndicator-test.js @@ -109,10 +109,7 @@ describe('ReactDefaultTransitionIndicator', () => { expect(root).toMatchRenderedOutput(
Hi
); await act(() => { - // TODO: This should not require a discrete update ideally but work for default too. - ReactNoop.discreteUpdates(() => { - update('Loading...'); - }); + update('Loading...'); React.startTransition(() => { update(''); root.render({promiseB}); From 59440424d05360cca32ca6f46ae33661f70d43e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 16:00:38 -0400 Subject: [PATCH 5/6] Implement Navigation API backed default indicator for DOM renderer (#33162) Stacked on #33160. By default, if `onDefaultTransitionIndicator` is not overridden, this will trigger a fake Navigation event using the Navigation API. This is intercepted to create an on-going navigation until we complete the Transition. Basically each default Transition is simulated as a Navigation. This triggers the native browser loading state (in Chrome at least). So now by default the browser spinner spins during a Transition if no other loading state is provided. Firefox and Safari hasn't shipped Navigation API yet and even in the flag Safari has, it doesn't actually trigger the native loading state. To ensures that you can still use other Navigations concurrently, we don't start our fake Navigation if there's one on-going already. Similarly if our fake Navigation gets interrupted by another. We wait for on-going ones to finish and then start a new fake one if we're supposed to be still pending. There might be other routers on the page that might listen to intercept Navigation Events. Typically you'd expect them not to trigger a refetch when navigating to the same state. However, if they want to detect this we provide the `"react-transition"` string in the `info` field for this purpose. --- .eslintrc.js | 2 + fixtures/flight/src/actions.js | 8 ++ .../view-transition/src/components/Page.js | 12 +- .../ReactDOMDefaultTransitionIndicator.js | 89 +++++++++++++ packages/react-dom/src/client/ReactDOMRoot.js | 6 +- scripts/flow/environment.js | 124 ++++++++++++++++++ scripts/rollup/validate/eslintrc.cjs.js | 1 + scripts/rollup/validate/eslintrc.cjs2015.js | 1 + scripts/rollup/validate/eslintrc.esm.js | 1 + scripts/rollup/validate/eslintrc.fb.js | 1 + scripts/rollup/validate/eslintrc.rn.js | 1 + 11 files changed, 240 insertions(+), 6 deletions(-) create mode 100644 packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js diff --git a/.eslintrc.js b/.eslintrc.js index 49846c1f5e9..c1eb5b34ebe 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -579,6 +579,7 @@ module.exports = { JSONValue: 'readonly', JSResourceReference: 'readonly', MouseEventHandler: 'readonly', + NavigateEvent: 'readonly', PropagationPhases: 'readonly', PropertyDescriptor: 'readonly', React$AbstractComponent: 'readonly', @@ -634,5 +635,6 @@ module.exports = { AsyncLocalStorage: 'readonly', async_hooks: 'readonly', globalThis: 'readonly', + navigation: 'readonly', }, }; diff --git a/fixtures/flight/src/actions.js b/fixtures/flight/src/actions.js index aa19871a9dc..0b9b9c315d6 100644 --- a/fixtures/flight/src/actions.js +++ b/fixtures/flight/src/actions.js @@ -2,7 +2,13 @@ import {setServerState} from './ServerState.js'; +async function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + export async function like() { + // Test loading state + await sleep(1000); setServerState('Liked!'); return new Promise((resolve, reject) => resolve('Liked')); } @@ -20,5 +26,7 @@ export async function greet(formData) { } export async function increment(n) { + // Test loading state + await sleep(1000); return n + 1; } diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 9744313c4f5..d7f7bc09836 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -18,6 +18,10 @@ import './Page.css'; import transitions from './Transitions.module.css'; +async function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + const a = (
@@ -106,7 +110,13 @@ export default function Page({url, navigate}) { document.body ) ) : ( - ); diff --git a/packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js b/packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js new file mode 100644 index 00000000000..8f1a32d826c --- /dev/null +++ b/packages/react-dom/src/client/ReactDOMDefaultTransitionIndicator.js @@ -0,0 +1,89 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export function defaultOnDefaultTransitionIndicator(): void | (() => void) { + if (typeof navigation !== 'object') { + // If the Navigation API is not available, then this is a noop. + return; + } + + let isCancelled = false; + let pendingResolve: null | (() => void) = null; + + function handleNavigate(event: NavigateEvent) { + if (event.canIntercept && event.info === 'react-transition') { + event.intercept({ + handler() { + return new Promise(resolve => (pendingResolve = resolve)); + }, + focusReset: 'manual', + scroll: 'manual', + }); + } + } + + function handleNavigateComplete() { + if (pendingResolve !== null) { + // If this was not our navigation completing, we were probably cancelled. + // We'll start a new one below. + pendingResolve(); + pendingResolve = null; + } + if (!isCancelled) { + // Some other navigation completed but we should still be running. + // Start another fake one to keep the loading indicator going. + startFakeNavigation(); + } + } + + // $FlowFixMe + navigation.addEventListener('navigate', handleNavigate); + // $FlowFixMe + navigation.addEventListener('navigatesuccess', handleNavigateComplete); + // $FlowFixMe + navigation.addEventListener('navigateerror', handleNavigateComplete); + + function startFakeNavigation() { + if (isCancelled) { + // We already stopped this Transition. + return; + } + if (navigation.transition) { + // There is an on-going Navigation already happening. Let's wait for it to + // finish before starting our fake one. + return; + } + // Trigger a fake navigation to the same page + const currentEntry = navigation.currentEntry; + if (currentEntry && currentEntry.url != null) { + navigation.navigate(currentEntry.url, { + state: currentEntry.getState(), + info: 'react-transition', // indicator to routers to ignore this navigation + history: 'replace', + }); + } + } + + // Delay the start a bit in case this is a fast navigation. + setTimeout(startFakeNavigation, 100); + + return function () { + isCancelled = true; + // $FlowFixMe + navigation.removeEventListener('navigate', handleNavigate); + // $FlowFixMe + navigation.removeEventListener('navigatesuccess', handleNavigateComplete); + // $FlowFixMe + navigation.removeEventListener('navigateerror', handleNavigateComplete); + if (pendingResolve !== null) { + pendingResolve(); + pendingResolve = null; + } + }; +} diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index ef2c9ddf193..97f4c835153 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -95,13 +95,9 @@ import { defaultOnCaughtError, defaultOnRecoverableError, } from 'react-reconciler/src/ReactFiberReconciler'; +import {defaultOnDefaultTransitionIndicator} from './ReactDOMDefaultTransitionIndicator'; import {ConcurrentRoot} from 'react-reconciler/src/ReactRootTags'; -function defaultOnDefaultTransitionIndicator(): void | (() => void) { - // TODO: Implement the default - return function () {}; -} - // $FlowFixMe[missing-this-annot] function ReactDOMRoot(internalRoot: FiberRoot) { this._internalRoot = internalRoot; diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js index c3fe40eeef3..d66ef65d9d3 100644 --- a/scripts/flow/environment.js +++ b/scripts/flow/environment.js @@ -429,3 +429,127 @@ declare const Bun: { input: string | $TypedArray | DataView | ArrayBuffer | SharedArrayBuffer, ): number, }; + +// Navigation API + +declare const navigation: Navigation; + +interface NavigationResult { + committed: Promise; + finished: Promise; +} + +declare class Navigation extends EventTarget { + entries(): NavigationHistoryEntry[]; + +currentEntry: NavigationHistoryEntry | null; + updateCurrentEntry(options: NavigationUpdateCurrentEntryOptions): void; + +transition: NavigationTransition | null; + + +canGoBack: boolean; + +canGoForward: boolean; + + navigate(url: string, options?: NavigationNavigateOptions): NavigationResult; + reload(options?: NavigationReloadOptions): NavigationResult; + + traverseTo(key: string, options?: NavigationOptions): NavigationResult; + back(options?: NavigationOptions): NavigationResult; + forward(options?: NavigationOptions): NavigationResult; + + onnavigate: ((this: Navigation, ev: NavigateEvent) => any) | null; + onnavigatesuccess: ((this: Navigation, ev: Event) => any) | null; + onnavigateerror: ((this: Navigation, ev: ErrorEvent) => any) | null; + oncurrententrychange: + | ((this: Navigation, ev: NavigationCurrentEntryChangeEvent) => any) + | null; + + // TODO: Implement addEventListener overrides. Doesn't seem like Flow supports this. +} + +declare class NavigationTransition { + +navigationType: NavigationTypeString; + +from: NavigationHistoryEntry; + +finished: Promise; +} + +interface NavigationHistoryEntryEventMap { + dispose: Event; +} + +interface NavigationHistoryEntry extends EventTarget { + +key: string; + +id: string; + +url: string | null; + +index: number; + +sameDocument: boolean; + + getState(): mixed; + + ondispose: ((this: NavigationHistoryEntry, ev: Event) => any) | null; + + // TODO: Implement addEventListener overrides. Doesn't seem like Flow supports this. +} + +declare var NavigationHistoryEntry: { + prototype: NavigationHistoryEntry, + new(): NavigationHistoryEntry, +}; + +type NavigationTypeString = 'reload' | 'push' | 'replace' | 'traverse'; + +interface NavigationUpdateCurrentEntryOptions { + state: mixed; +} + +interface NavigationOptions { + info?: mixed; +} + +interface NavigationNavigateOptions extends NavigationOptions { + state?: mixed; + history?: 'auto' | 'push' | 'replace'; +} + +interface NavigationReloadOptions extends NavigationOptions { + state?: mixed; +} + +declare class NavigationCurrentEntryChangeEvent extends Event { + constructor(type: string, eventInit?: any): void; + + +navigationType: NavigationTypeString | null; + +from: NavigationHistoryEntry; +} + +declare class NavigateEvent extends Event { + constructor(type: string, eventInit?: any): void; + + +navigationType: NavigationTypeString; + +canIntercept: boolean; + +userInitiated: boolean; + +hashChange: boolean; + +hasUAVisualTransition: boolean; + +destination: NavigationDestination; + +signal: AbortSignal; + +formData: FormData | null; + +downloadRequest: string | null; + +info?: mixed; + + intercept(options?: NavigationInterceptOptions): void; + scroll(): void; +} + +interface NavigationInterceptOptions { + handler?: () => Promise; + focusReset?: 'after-transition' | 'manual'; + scroll?: 'after-transition' | 'manual'; +} + +declare class NavigationDestination { + +url: string; + +key: string | null; + +id: string | null; + +index: number; + +sameDocument: boolean; + + getState(): mixed; +} diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 88d17772d7b..65fd6129904 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index 8e87c8dbe02..fa0b471330f 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -33,6 +33,7 @@ module.exports = { globalThis: 'readonly', FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', __REACT_DEVTOOLS_GLOBAL_HOOK__: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.esm.js b/scripts/rollup/validate/eslintrc.esm.js index 8b4bba35796..a5ea7afb972 100644 --- a/scripts/rollup/validate/eslintrc.esm.js +++ b/scripts/rollup/validate/eslintrc.esm.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index f0602e79e50..afee2f1199e 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index 052edabdc0f..2420898bebe 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -35,6 +35,7 @@ module.exports = { FinalizationRegistry: 'readonly', ScrollTimeline: 'readonly', + navigation: 'readonly', // Vendor specific MSApp: 'readonly', From 3a5b326d8180f005a10e34a07ded6d5632efe337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 13 May 2025 16:10:28 -0400 Subject: [PATCH 6/6] [Fiber] Trigger default indicator for isomorphic async actions with no root associated (#33190) Stacked on #33160, #33162, #33186 and #33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done. --- .../view-transition/src/components/Page.js | 2 +- .../src/ReactFiberAsyncAction.js | 102 +++++++++++++- .../src/ReactFiberReconciler.js | 7 +- .../src/ReactFiberRootScheduler.js | 56 +++++--- .../ReactDefaultTransitionIndicator-test.js | 127 +++++++++++++++++- 5 files changed, 274 insertions(+), 20 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index d7f7bc09836..db2cd0aff08 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -113,8 +113,8 @@ export default function Page({url, navigate}) {