diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index cda1b2b94deb2..cc217d96e78c6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -36,7 +36,10 @@ import type { } from './ReactFiberCacheComponent.new'; import type {UpdateQueue} from './ReactFiberClassUpdateQueue.new'; import type {RootState} from './ReactFiberRoot.new'; -import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent.new'; +import type { + TracingMarkerInstance, + TracingMarkerQueue, +} from './ReactFiberTracingMarkerComponent.new'; import { enableCPUSuspense, enableUseMutableSource, @@ -968,20 +971,23 @@ function updateTracingMarkerComponent( if (!enableTransitionTracing) { return null; } + const prevName = current !== null ? current.memoizedProps.name : null; + const nextName = workInProgress.pendingProps.name; - // TODO: (luna) Only update the tracing marker if it's newly rendered or it's name changed. + // Update the tracing marker if it's newly rendered or it's name changed. // A tracing marker is only associated with the transitions that rendered // or updated it, so we can create a new set of transitions each time - if (current === null) { - const currentTransitions = getPendingTransitions(); - if (currentTransitions !== null) { - const markerInstance: TracingMarkerInstance = { - transitions: new Set(currentTransitions), - pendingSuspenseBoundaries: new Map(), - hasUpdate: false, - }; - workInProgress.stateNode = markerInstance; - } + // TODO: If the name changed, all current transitions on the tracing marker + // are canceled + if (current === null || prevName !== nextName) { + const transitions = getPendingTransitions(); + // We store the new transitions associated with the Tracing Marker + // on the update queue because we can't modify the instance object in the + // render phase + const updateQueue: TracingMarkerQueue | null = { + transitions: transitions ? new Set(transitions) : null, + }; + workInProgress.updateQueue = updateQueue; } const instance: TracingMarkerInstance | null = workInProgress.stateNode; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 22b23297d69f8..c8bc83d25a312 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -30,7 +30,11 @@ import type { import type {HookFlags} from './ReactHookEffectTags'; import type {Cache} from './ReactFiberCacheComponent.new'; import type {RootState} from './ReactFiberRoot.new'; -import type {Transition} from './ReactFiberTracingMarkerComponent.new'; +import type { + Transition, + TracingMarkerQueue, + TracingMarkerInstance, +} from './ReactFiberTracingMarkerComponent.new'; import { enableCreateEventHandleAPI, @@ -2451,6 +2455,29 @@ function commitMutationEffectsOnFiber( } return; } + case TracingMarkerComponent: { + recursivelyTraverseMutationEffects(root, finishedWork, lanes); + commitReconciliationEffects(finishedWork); + + if (enableTransitionTracing) { + // If there is an update on the Tracing Marker, this means that any + // previous transitions associated with the marker are cancelled + // We want to clear them (and create a new pending suspense boundaries + // map) before we modify the suspense boundaries map so the information + // on the tracing marker is accurate in the passive phase. + const queue: TracingMarkerQueue | null = (finishedWork.updateQueue: any); + const instance: TracingMarkerInstance = finishedWork.stateNode; + if (queue !== null) { + instance.transitions = queue.transitions; + instance.hasUpdate = queue.transitions !== null; + instance.pendingSuspenseBoundaries = + queue.transitions !== null ? new Map() : null; + + finishedWork.updateQueue = null; + } + } + return; + } default: { recursivelyTraverseMutationEffects(root, finishedWork, lanes); commitReconciliationEffects(finishedWork); @@ -3012,18 +3039,20 @@ function commitPassiveMountOnFiber( // Get the transitions that were initiatized during the render // and add a start transition callback for each of them const instance = finishedWork.stateNode; - if ( - instance.pendingSuspenseBoundaries === null || - instance.pendingSuspenseBoundaries.size === 0 - ) { - instance.transitions.forEach(transition => { - addMarkerCompleteCallbackToPendingTransition({ - transition, - name: finishedWork.memoizedProps.name, + if (instance.transitions !== null) { + if ( + instance.pendingSuspenseBoundaries === null || + instance.pendingSuspenseBoundaries.size === 0 + ) { + instance.transitions.forEach(transition => { + addMarkerCompleteCallbackToPendingTransition({ + transition, + name: finishedWork.memoizedProps.name, + }); }); - }); - instance.transitions = null; - instance.pendingSuspenseBoundaries = null; + instance.transitions = null; + instance.pendingSuspenseBoundaries = null; + } } } break; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index 058a14d802aaa..10f083b96d2d4 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -1603,10 +1603,15 @@ function completeWork( if (instance !== null) { popMarkerInstance(workInProgress); } + + if (workInProgress.updateQueue !== null) { + markUpdate(workInProgress); + } + bubbleProperties(workInProgress); if ( - current === null || + workInProgress.updateQueue !== null || (workInProgress.subtreeFlags & Visibility) !== NoFlags ) { // If any of our suspense children toggle visibility, this means that diff --git a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js index 938e3f23b9865..8c0b53c295759 100644 --- a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js @@ -51,6 +51,10 @@ export type TracingMarkerInstance = {| hasUpdate: boolean, |}; +export type TracingMarkerQueue = {| + transitions: Set | null, +|}; + export type PendingSuspenseBoundaries = Map; export function processTransitionCallbacks( diff --git a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js index fbfcdb9b08d9a..e94aee6a25b41 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js @@ -1061,7 +1061,7 @@ describe('ReactInteractionTracing', () => { }); // @gate enableTransitionTracing - it.skip('marker interaction cancelled when name changes', async () => { + it('marker interaction cancelled when name changes', async () => { const transitionCallbacks = { onTransitionStart: (name, startTime) => { Scheduler.unstable_yieldValue( @@ -1124,7 +1124,6 @@ describe('ReactInteractionTracing', () => { ReactNoop.expire(1000); await advanceTimers(1000); setMarkerNameFn(); - expect(Scheduler).toFlushAndYield(['Suspend [Page Two]', 'Loading...']); ReactNoop.expire(1000); await advanceTimers(1000); @@ -1139,7 +1138,7 @@ describe('ReactInteractionTracing', () => { }); // @gate enableTransitionTracing - it.skip('marker changes to new interaction when name changes', async () => { + it('marker changes to new interaction when name changes', async () => { const transitionCallbacks = { onTransitionStart: (name, startTime) => { Scheduler.unstable_yieldValue( @@ -1193,30 +1192,34 @@ describe('ReactInteractionTracing', () => { expect(Scheduler).toFlushAndYield(['Page One']); startTransition(() => navigateToPageTwo(), {name: 'page transition'}); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ 'Suspend [Page Two]', 'Loading...', 'onTransitionStart(page transition, 1000)', ]); + startTransition(() => setMarkerNameFn(), {name: 'marker transition'}); ReactNoop.expire(1000); await advanceTimers(1000); - startTransition(() => setMarkerNameFn(), {name: 'marker transition'}); expect(Scheduler).toFlushAndYield([ 'Suspend [Page Two]', 'Loading...', 'onTransitionStart(marker transition, 2000)', + 'onMarkerComplete(marker transition, new marker, 2000, 3000)', + 'onTransitionComplete(marker transition, 2000, 3000)', ]); + resolveText('Page Two'); ReactNoop.expire(1000); await advanceTimers(1000); - resolveText('Page Two'); // Marker complete is not called because the marker name changed expect(Scheduler).toFlushAndYield([ 'Page Two', - 'onMarkerComplete(new marker, 2000, 3000)', - 'onTransitionComplete(page transition, 1000, 3000)', + 'onTransitionComplete(page transition, 1000, 4000)', ]); }); });