From 110f762d1af3c0972fdd3a2caacf5500e49396c2 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 17 Apr 2023 16:24:22 -0400 Subject: [PATCH] Revert "Delete unused `eventTimes` Fiber field (#26599)" This reverts commit 58742c21b8c3237e8b66c7df4e200504846a01ae. --- .../src/ReactFiberBeginWork.js | 10 ++++- .../src/ReactFiberClassComponent.js | 15 +++++-- .../src/ReactFiberCommitWork.js | 6 +-- .../react-reconciler/src/ReactFiberHooks.js | 13 ++++-- .../src/ReactFiberHotReloading.js | 4 +- .../react-reconciler/src/ReactFiberLane.js | 33 +++++++++++++- .../src/ReactFiberReconciler.js | 31 +++++++------ .../react-reconciler/src/ReactFiberRoot.js | 1 + .../src/ReactFiberWorkLoop.js | 43 ++++++++++++++++--- .../src/ReactInternalTypes.js | 1 + .../__tests__/ReactProfiler-test.internal.js | 13 ++++-- 11 files changed, 133 insertions(+), 37 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 043181597d7e7..7b5b2f72b1e0c 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -142,6 +142,7 @@ import { OffscreenLane, DefaultHydrationLane, SomeRetryLane, + NoTimestamp, includesSomeLane, laneToLanes, removeLanes, @@ -2875,8 +2876,15 @@ function updateDehydratedSuspenseComponent( // is one of the very rare times where we mutate the current tree // during the render phase. suspenseState.retryLane = attemptHydrationAtLane; + // TODO: Ideally this would inherit the event time of the current render + const eventTime = NoTimestamp; enqueueConcurrentRenderForLane(current, attemptHydrationAtLane); - scheduleUpdateOnFiber(root, current, attemptHydrationAtLane); + scheduleUpdateOnFiber( + root, + current, + attemptHydrationAtLane, + eventTime, + ); // Throw a special object that signals to the work loop that it should // interrupt the current render. diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index b071808a9117d..7aa3d934f3b12 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -64,7 +64,11 @@ import { emptyContextObject, } from './ReactFiberContext'; import {readContext, checkIfContextChanged} from './ReactFiberNewContext'; -import {requestUpdateLane, scheduleUpdateOnFiber} from './ReactFiberWorkLoop'; +import { + requestEventTime, + requestUpdateLane, + scheduleUpdateOnFiber, +} from './ReactFiberWorkLoop'; import {logForceUpdateScheduled, logStateUpdateScheduled} from './DebugTracing'; import { markForceUpdateScheduled, @@ -209,7 +213,8 @@ const classComponentUpdater = { const root = enqueueUpdate(fiber, update, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); entangleTransitions(root, fiber, lane); } @@ -243,7 +248,8 @@ const classComponentUpdater = { const root = enqueueUpdate(fiber, update, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); entangleTransitions(root, fiber, lane); } @@ -277,7 +283,8 @@ const classComponentUpdater = { const root = enqueueUpdate(fiber, update, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); entangleTransitions(root, fiber, lane); } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b540929ce0352..9ea1d95796267 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -18,7 +18,7 @@ import type { } from './ReactFiberConfig'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; -import {SyncLane} from './ReactFiberLane'; +import {NoTimestamp, SyncLane} from './ReactFiberLane'; import type {SuspenseState, RetryQueue} from './ReactFiberSuspenseComponent'; import type {UpdateQueue} from './ReactFiberClassUpdateQueue'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; @@ -2415,7 +2415,7 @@ export function detachOffscreenInstance(instance: OffscreenInstance): void { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { instance._pendingVisibility |= OffscreenDetached; - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } @@ -2435,7 +2435,7 @@ export function attachOffscreenInstance(instance: OffscreenInstance): void { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { instance._pendingVisibility &= ~OffscreenDetached; - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 4be1994114a32..369a662cb61b1 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -70,6 +70,7 @@ import { isTransitionLane, markRootEntangled, markRootMutableRead, + NoTimestamp, } from './ReactFiberLane'; import { ContinuousEventPriority, @@ -100,6 +101,7 @@ import { getWorkInProgressRootRenderLanes, scheduleUpdateOnFiber, requestUpdateLane, + requestEventTime, markSkippedUpdateLanes, isInvalidExecutionContextForEventFunction, } from './ReactFiberWorkLoop'; @@ -1835,7 +1837,7 @@ function checkIfSnapshotChanged(inst: StoreInstance): boolean { function forceStoreRerender(fiber: Fiber) { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } @@ -2556,7 +2558,8 @@ function refreshCache(fiber: Fiber, seedKey: ?() => T, seedValue: T): void { const refreshUpdate = createLegacyQueueUpdate(lane); const root = enqueueLegacyQueueUpdate(provider, refreshUpdate, lane); if (root !== null) { - scheduleUpdateOnFiber(root, provider, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, provider, lane, eventTime); entangleLegacyQueueTransitions(root, provider, lane); } @@ -2620,7 +2623,8 @@ function dispatchReducerAction( } else { const root = enqueueConcurrentHookUpdate(fiber, queue, update, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); entangleTransitionUpdate(root, queue, lane); } } @@ -2702,7 +2706,8 @@ function dispatchSetState( const root = enqueueConcurrentHookUpdate(fiber, queue, update, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); entangleTransitionUpdate(root, queue, lane); } } diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.js b/packages/react-reconciler/src/ReactFiberHotReloading.js index 3bc3755bb957a..ffb3ae8a0ee1b 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.js @@ -23,7 +23,7 @@ import { import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates'; import {updateContainer} from './ReactFiberReconciler'; import {emptyContextObject} from './ReactFiberContext'; -import {SyncLane} from './ReactFiberLane'; +import {SyncLane, NoTimestamp} from './ReactFiberLane'; import { ClassComponent, FunctionComponent, @@ -328,7 +328,7 @@ function scheduleFibersWithFamiliesRecursively( if (needsRemount || needsRender) { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } if (child !== null && !needsRemount) { diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 835bf01193423..45c8810c7f477 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -319,6 +319,25 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { return nextLanes; } +export function getMostRecentEventTime(root: FiberRoot, lanes: Lanes): number { + const eventTimes = root.eventTimes; + + let mostRecentEventTime = NoTimestamp; + while (lanes > 0) { + const index = pickArbitraryLaneIndex(lanes); + const lane = 1 << index; + + const eventTime = eventTimes[index]; + if (eventTime > mostRecentEventTime) { + mostRecentEventTime = eventTime; + } + + lanes &= ~lane; + } + + return mostRecentEventTime; +} + function computeExpirationTime(lane: Lane, currentTime: number) { switch (lane) { case SyncHydrationLane: @@ -580,7 +599,11 @@ export function createLaneMap(initial: T): LaneMap { return laneMap; } -export function markRootUpdated(root: FiberRoot, updateLane: Lane) { +export function markRootUpdated( + root: FiberRoot, + updateLane: Lane, + eventTime: number, +) { root.pendingLanes |= updateLane; // If there are any suspended transitions, it's possible this new update @@ -599,6 +622,12 @@ export function markRootUpdated(root: FiberRoot, updateLane: Lane) { root.suspendedLanes = NoLanes; root.pingedLanes = NoLanes; } + + const eventTimes = root.eventTimes; + const index = laneToIndex(updateLane); + // We can always overwrite an existing timestamp because we prefer the most + // recent event, and we assume time is monotonically increasing. + eventTimes[index] = eventTime; } export function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) { @@ -643,6 +672,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { root.errorRecoveryDisabledLanes &= remainingLanes; const entanglements = root.entanglements; + const eventTimes = root.eventTimes; const expirationTimes = root.expirationTimes; const hiddenUpdates = root.hiddenUpdates; @@ -653,6 +683,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { const lane = 1 << index; entanglements[index] = NoLanes; + eventTimes[index] = NoTimestamp; expirationTimes[index] = NoTimestamp; const hiddenUpdatesForLane = hiddenUpdates[index]; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 72e020f945c64..5ac1ecaa4ea74 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -56,6 +56,7 @@ import { onScheduleRoot, } from './ReactFiberDevToolsHook'; import { + requestEventTime, requestUpdateLane, scheduleUpdateOnFiber, scheduleInitialHydrationOnRoot, @@ -83,6 +84,7 @@ import {StrictLegacyMode} from './ReactTypeOfMode'; import { SyncLane, SelectiveHydrationLane, + NoTimestamp, getHighestPriorityPendingLanes, higherPriorityLane, } from './ReactFiberLane'; @@ -309,8 +311,9 @@ export function createHydrationContainer( const update = createUpdate(lane); update.callback = callback !== undefined && callback !== null ? callback : null; + const eventTime = requestEventTime(); enqueueUpdate(current, update, lane); - scheduleInitialHydrationOnRoot(root, lane); + scheduleInitialHydrationOnRoot(root, lane, eventTime); return root; } @@ -376,7 +379,8 @@ export function updateContainer( const root = enqueueUpdate(current, update, lane); if (root !== null) { - scheduleUpdateOnFiber(root, current, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, current, lane, eventTime); entangleTransitions(root, current, lane); } @@ -423,7 +427,8 @@ export function attemptSynchronousHydration(fiber: Fiber): void { flushSync(() => { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, SyncLane, eventTime); } }); // If we're still blocked after this, we need to increase @@ -466,7 +471,8 @@ export function attemptContinuousHydration(fiber: Fiber): void { const lane = SelectiveHydrationLane; const root = enqueueConcurrentRenderForLane(fiber, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); } markRetryLaneIfNotHydrated(fiber, lane); } @@ -480,7 +486,8 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void { const lane = requestUpdateLane(fiber); const root = enqueueConcurrentRenderForLane(fiber, lane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, lane); + const eventTime = requestEventTime(); + scheduleUpdateOnFiber(root, fiber, lane, eventTime); } markRetryLaneIfNotHydrated(fiber, lane); } @@ -659,7 +666,7 @@ if (__DEV__) { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } }; @@ -683,7 +690,7 @@ if (__DEV__) { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } }; @@ -708,7 +715,7 @@ if (__DEV__) { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } } }; @@ -721,7 +728,7 @@ if (__DEV__) { } const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } }; overridePropsDeletePath = (fiber: Fiber, path: Array) => { @@ -731,7 +738,7 @@ if (__DEV__) { } const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } }; overridePropsRenamePath = ( @@ -745,14 +752,14 @@ if (__DEV__) { } const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } }; scheduleUpdate = (fiber: Fiber) => { const root = enqueueConcurrentRenderForLane(fiber, SyncLane); if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + scheduleUpdateOnFiber(root, fiber, SyncLane, NoTimestamp); } }; diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 7fd4802d0cc79..35daaf7c10dc4 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -66,6 +66,7 @@ function FiberRootNode( this.next = null; this.callbackNode = null; this.callbackPriority = NoLane; + this.eventTimes = createLaneMap(NoLanes); this.expirationTimes = createLaneMap(NoTimestamp); this.pendingLanes = NoLanes; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6f00533d3004d..d75cc2c47ff15 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -130,6 +130,7 @@ import { NoLanes, NoLane, SyncLane, + NoTimestamp, claimNextTransitionLane, claimNextRetryLane, includesSyncLane, @@ -583,6 +584,10 @@ const NESTED_PASSIVE_UPDATE_LIMIT = 50; let nestedPassiveUpdateCount: number = 0; let rootWithPassiveNestedUpdates: FiberRoot | null = null; +// If two updates are scheduled within the same event, we should treat their +// event times as simultaneous, even if the actual clock time has advanced +// between the first and second call. +let currentEventTime: number = NoTimestamp; let currentEventTransitionLane: Lanes = NoLanes; let isRunningInsertionEffect = false; @@ -599,6 +604,21 @@ export function isWorkLoopSuspendedOnData(): boolean { return workInProgressSuspendedReason === SuspendedOnData; } +export function requestEventTime(): number { + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { + // We're inside React, so it's fine to read the actual time. + return now(); + } + // We're not inside React, so we may be in the middle of a browser event. + if (currentEventTime !== NoTimestamp) { + // Use the same start time for all updates until we enter React again. + return currentEventTime; + } + // This is the first update since React yielded. Compute a new start time. + currentEventTime = now(); + return currentEventTime; +} + export function getCurrentTime(): number { return now(); } @@ -687,6 +707,7 @@ export function scheduleUpdateOnFiber( root: FiberRoot, fiber: Fiber, lane: Lane, + eventTime: number, ) { if (__DEV__) { if (isRunningInsertionEffect) { @@ -716,7 +737,7 @@ export function scheduleUpdateOnFiber( } // Mark that the root has a pending update. - markRootUpdated(root, lane); + markRootUpdated(root, lane, eventTime); if ( (executionContext & RenderContext) !== NoLanes && @@ -817,7 +838,11 @@ export function scheduleUpdateOnFiber( } } -export function scheduleInitialHydrationOnRoot(root: FiberRoot, lane: Lane) { +export function scheduleInitialHydrationOnRoot( + root: FiberRoot, + lane: Lane, + eventTime: number, +) { // This is a special fork of scheduleUpdateOnFiber that is only used to // schedule the initial hydration of a root that has just been created. Most // of the stuff in scheduleUpdateOnFiber can be skipped. @@ -829,7 +854,7 @@ export function scheduleInitialHydrationOnRoot(root: FiberRoot, lane: Lane) { // match what was rendered on the server. const current = root.current; current.lanes = lane; - markRootUpdated(root, lane); + markRootUpdated(root, lane, eventTime); ensureRootIsScheduled(root); } @@ -849,6 +874,9 @@ export function performConcurrentWorkOnRoot( resetNestedUpdateFlag(); } + // Since we know we're in a React event, we can clear the current + // event time. The next update will compute a new event time. + currentEventTime = NoTimestamp; currentEventTransitionLane = NoLanes; if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { @@ -3285,8 +3313,9 @@ function captureCommitPhaseErrorOnRoot( const errorInfo = createCapturedValueAtFiber(error, sourceFiber); const update = createRootErrorUpdate(rootFiber, errorInfo, (SyncLane: Lane)); const root = enqueueUpdate(rootFiber, update, (SyncLane: Lane)); + const eventTime = requestEventTime(); if (root !== null) { - markRootUpdated(root, SyncLane); + markRootUpdated(root, SyncLane, eventTime); ensureRootIsScheduled(root); } } @@ -3327,8 +3356,9 @@ export function captureCommitPhaseError( (SyncLane: Lane), ); const root = enqueueUpdate(fiber, update, (SyncLane: Lane)); + const eventTime = requestEventTime(); if (root !== null) { - markRootUpdated(root, SyncLane); + markRootUpdated(root, SyncLane, eventTime); ensureRootIsScheduled(root); } return; @@ -3463,9 +3493,10 @@ function retryTimedOutBoundary(boundaryFiber: Fiber, retryLane: Lane) { retryLane = requestRetryLane(boundaryFiber); } // TODO: Special case idle priority? + const eventTime = requestEventTime(); const root = enqueueConcurrentRenderForLane(boundaryFiber, retryLane); if (root !== null) { - markRootUpdated(root, retryLane); + markRootUpdated(root, retryLane, eventTime); ensureRootIsScheduled(root); } } diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 281186c53de0d..c7264083172f0 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -248,6 +248,7 @@ type BaseFiberRootProperties = { // task that the root will work on. callbackNode: any, callbackPriority: Lane, + eventTimes: LaneMap, expirationTimes: LaneMap, hiddenUpdates: LaneMap | null>, diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index d728d002587b7..9ae071f360617 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -268,13 +268,14 @@ describe(`onRender`, () => { , ); - // Restore original mock - jest.mock('scheduler', () => jest.requireActual('scheduler/unstable_mock')); - // TODO: unstable_now is called by more places than just the profiler. // Rewrite this test so it's less fragile. if (gate(flags => flags.enableDeferRootSchedulingToMicrotask)) { - assertLog(['read current time', 'read current time']); + assertLog([ + 'read current time', + 'read current time', + 'read current time', + ]); } else { assertLog([ 'read current time', @@ -282,8 +283,12 @@ describe(`onRender`, () => { 'read current time', 'read current time', 'read current time', + 'read current time', ]); } + + // Restore original mock + jest.mock('scheduler', () => jest.requireActual('scheduler/unstable_mock')); }); it('does not report work done on a sibling', async () => {