From dad5424306fa07442e9ac595fd12060c32b53368 Mon Sep 17 00:00:00 2001 From: Luna Date: Thu, 30 Jun 2022 17:00:54 -0400 Subject: [PATCH] add transition progress callback --- .../react-reconciler/src/ReactFiber.new.js | 1 + .../src/ReactFiberBeginWork.new.js | 1 + .../src/ReactFiberCommitWork.new.js | 54 +-- .../src/ReactFiberOffscreenComponent.js | 3 +- .../ReactFiberTracingMarkerComponent.new.js | 26 +- .../src/ReactFiberWorkLoop.new.js | 25 ++ .../__tests__/ReactTransitionTracing-test.js | 329 +++++++++++++++++- 7 files changed, 410 insertions(+), 29 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 91d1211c20720..f868be04c4245 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -771,6 +771,7 @@ export function createFiberFromTracingMarker( const tracingMarkerInstance: TracingMarkerInstance = { transitions: null, pendingSuspenseBoundaries: null, + hasUpdate: false, }; fiber.stateNode = tracingMarkerInstance; return fiber; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index c3bf502ac6180..cda1b2b94deb2 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -978,6 +978,7 @@ function updateTracingMarkerComponent( const markerInstance: TracingMarkerInstance = { transitions: new Set(currentTransitions), pendingSuspenseBoundaries: new Map(), + hasUpdate: false, }; workInProgress.stateNode = markerInstance; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index e5cfebea5f88c..22b23297d69f8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -138,6 +138,7 @@ import { enqueuePendingPassiveProfilerEffect, restorePendingUpdaters, addTransitionStartCallbackToPendingTransition, + addTransitionProgressCallbackToPendingTransition, addTransitionCompleteCallbackToPendingTransition, addMarkerCompleteCallbackToPendingTransition, setIsRunningInsertionEffect, @@ -1110,10 +1111,17 @@ function commitTransitionProgress(offscreenFiber: Fiber) { // The suspense boundaries was just hidden. Add the boundary // to the pending boundary set if it's there if (pendingMarkers !== null) { - pendingMarkers.forEach(pendingBoundaries => { - pendingBoundaries.set(offscreenInstance, { - name, - }); + pendingMarkers.forEach(markerInstance => { + const pendingBoundaries = markerInstance.pendingSuspenseBoundaries; + if ( + pendingBoundaries !== null && + !pendingBoundaries.has(offscreenInstance) + ) { + pendingBoundaries.set(offscreenInstance, { + name, + }); + markerInstance.hasUpdate = true; + } }); } } else if (wasHidden && !isHidden) { @@ -1121,9 +1129,14 @@ function commitTransitionProgress(offscreenFiber: Fiber) { // the boundary from the pending suspense boundaries set // if it's there if (pendingMarkers !== null) { - pendingMarkers.forEach(pendingBoundaries => { - if (pendingBoundaries.has(offscreenInstance)) { + pendingMarkers.forEach(markerInstance => { + const pendingBoundaries = markerInstance.pendingSuspenseBoundaries; + if ( + pendingBoundaries !== null && + pendingBoundaries.has(offscreenInstance) + ) { pendingBoundaries.delete(offscreenInstance); + markerInstance.hasUpdate = true; } }); } @@ -2867,17 +2880,20 @@ function commitPassiveMountOnFiber( clearTransitionsForLanes(finishedRoot, committedLanes); } - incompleteTransitions.forEach( - ({pendingSuspenseBoundaries}, transition) => { - if ( - pendingSuspenseBoundaries === null || - pendingSuspenseBoundaries.size === 0 - ) { - addTransitionCompleteCallbackToPendingTransition(transition); - incompleteTransitions.delete(transition); - } - }, - ); + incompleteTransitions.forEach((markerInstance, transition) => { + const pendingBoundaries = markerInstance.pendingSuspenseBoundaries; + if (markerInstance.hasUpdate) { + addTransitionProgressCallbackToPendingTransition({ + transition, + pending: pendingBoundaries || [], + }); + markerInstance.hasUpdate = false; + } + if (pendingBoundaries === null || pendingBoundaries.size === 0) { + addTransitionCompleteCallbackToPendingTransition(transition); + incompleteTransitions.delete(transition); + } + }); clearTransitionsForLanes(finishedRoot, committedLanes); } @@ -2954,9 +2970,7 @@ function commitPassiveMountOnFiber( instance.pendingMarkers = new Set(); } - instance.pendingMarkers.add( - markerInstance.pendingSuspenseBoundaries, - ); + instance.pendingMarkers.add(markerInstance); } }); } diff --git a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js index 0c989dd53fe67..ca1a3938410f7 100644 --- a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js +++ b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js @@ -12,7 +12,6 @@ import type {Lanes} from './ReactFiberLane.old'; import type {SpawnedCachePool} from './ReactFiberCacheComponent.new'; import type { Transition, - PendingSuspenseBoundaries, TracingMarkerInstance, } from './ReactFiberTracingMarkerComponent.new'; @@ -45,7 +44,7 @@ export type OffscreenQueue = {| export type OffscreenInstance = {| isHidden: boolean, - pendingMarkers: Set | null, + pendingMarkers: Set | null, transitions: Set | null, retryCache: WeakSet | Set | null, |}; diff --git a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js index e7522fd93fa4a..938e3f23b9865 100644 --- a/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js @@ -22,8 +22,14 @@ export type MarkerTransition = { name: string, }; +export type TransitionProgress = { + transition: Transition, + pending: PendingSuspenseBoundaries, +}; + export type PendingTransitionCallbacks = { transitionStart: Array | null, + transitionProgress: Array | null, transitionComplete: Array | null, markerComplete: Array | null, }; @@ -42,6 +48,7 @@ export type BatchConfigTransition = { export type TracingMarkerInstance = {| pendingSuspenseBoundaries: PendingSuspenseBoundaries | null, transitions: Set | null, + hasUpdate: boolean, |}; export type PendingSuspenseBoundaries = Map; @@ -76,6 +83,19 @@ export function processTransitionCallbacks( }); } + const transitionProgress = pendingTransitions.transitionProgress; + const onTransitionProgress = callbacks.onTransitionProgress; + if (onTransitionProgress != null && transitionProgress !== null) { + transitionProgress.forEach(({transition, pending}) => { + onTransitionProgress( + transition.name, + transition.startTime, + endTime, + Array.from(pending.values()), + ); + }); + } + const transitionComplete = pendingTransitions.transitionComplete; if (transitionComplete !== null) { transitionComplete.forEach(transition => { @@ -117,10 +137,12 @@ export function pushRootMarkerInstance(workInProgress: Fiber): void { if (transitions !== null) { transitions.forEach(transition => { if (!root.incompleteTransitions.has(transition)) { - root.incompleteTransitions.set(transition, { + const markerInstance: TracingMarkerInstance = { transitions: new Set([transition]), pendingSuspenseBoundaries: null, - }); + hasUpdate: true, + }; + root.incompleteTransitions.set(transition, markerInstance); } }); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index f3c141d76cf54..f17f5e0bc9f51 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -18,6 +18,7 @@ import type { PendingTransitionCallbacks, MarkerTransition, Transition, + TransitionProgress, } from './ReactFiberTracingMarkerComponent.new'; import type {OffscreenInstance} from './ReactFiberOffscreenComponent'; @@ -339,6 +340,7 @@ export function addTransitionStartCallbackToPendingTransition( if (currentPendingTransitionCallbacks === null) { currentPendingTransitionCallbacks = { transitionStart: [], + transitionProgress: null, transitionComplete: null, markerComplete: null, }; @@ -359,6 +361,7 @@ export function addMarkerCompleteCallbackToPendingTransition( if (currentPendingTransitionCallbacks === null) { currentPendingTransitionCallbacks = { transitionStart: null, + transitionProgress: null, transitionComplete: null, markerComplete: [], }; @@ -372,6 +375,27 @@ export function addMarkerCompleteCallbackToPendingTransition( } } +export function addTransitionProgressCallbackToPendingTransition( + transition: TransitionProgress, +) { + if (enableTransitionTracing) { + if (currentPendingTransitionCallbacks === null) { + currentPendingTransitionCallbacks = { + transitionStart: null, + transitionProgress: [], + transitionComplete: null, + markerComplete: null, + }; + } + + if (currentPendingTransitionCallbacks.transitionProgress === null) { + currentPendingTransitionCallbacks.transitionProgress = []; + } + + currentPendingTransitionCallbacks.transitionProgress.push(transition); + } +} + export function addTransitionCompleteCallbackToPendingTransition( transition: Transition, ) { @@ -379,6 +403,7 @@ export function addTransitionCompleteCallbackToPendingTransition( if (currentPendingTransitionCallbacks === null) { currentPendingTransitionCallbacks = { transitionStart: null, + transitionProgress: null, transitionComplete: [], markerComplete: null, }; diff --git a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js index 1926f47fb64d3..fbfcdb9b08d9a 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js @@ -168,6 +168,12 @@ describe('ReactInteractionTracing', () => { `onTransitionStart(${name}, ${startTime})`, ); }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, onTransitionComplete: (name, startTime, endTime) => { Scheduler.unstable_yieldValue( `onTransitionComplete(${name}, ${startTime}, ${endTime})`, @@ -206,6 +212,7 @@ describe('ReactInteractionTracing', () => { expect(Scheduler).toFlushAndYield([ 'Page Two', 'onTransitionStart(page transition, 1000)', + 'onTransitionProgress(page transition, 1000, 2000, [])', 'onTransitionComplete(page transition, 1000, 2000)', ]); }); @@ -283,6 +290,12 @@ describe('ReactInteractionTracing', () => { `onTransitionStart(${name}, ${startTime})`, ); }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, onTransitionComplete: (name, startTime, endTime) => { Scheduler.unstable_yieldValue( `onTransitionComplete(${name}, ${startTime}, ${endTime})`, @@ -301,7 +314,7 @@ describe('ReactInteractionTracing', () => { {navigate ? ( } - name="suspense page"> + unstable_name="suspense page"> ) : ( @@ -330,6 +343,7 @@ describe('ReactInteractionTracing', () => { 'Suspend [Page Two]', 'Loading...', 'onTransitionStart(page transition, 1000)', + 'onTransitionProgress(page transition, 1000, 2000, [suspense page])', ]); ReactNoop.expire(1000); @@ -338,6 +352,7 @@ describe('ReactInteractionTracing', () => { expect(Scheduler).toFlushAndYield([ 'Page Two', + 'onTransitionProgress(page transition, 1000, 3000, [])', 'onTransitionComplete(page transition, 1000, 3000)', ]); }); @@ -351,6 +366,12 @@ describe('ReactInteractionTracing', () => { `onTransitionStart(${name}, ${startTime})`, ); }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, onTransitionComplete: (name, startTime, endTime) => { Scheduler.unstable_yieldValue( `onTransitionComplete(${name}, ${startTime}, ${endTime})`, @@ -377,13 +398,15 @@ describe('ReactInteractionTracing', () => { {navigate ? ( <> {showText ? ( - }> + }> ) : null} } - name="suspense page"> + unstable_name="suspense page"> @@ -410,6 +433,7 @@ describe('ReactInteractionTracing', () => { 'Suspend [Page Two]', 'Loading...', 'onTransitionStart(page transition, 1000)', + 'onTransitionProgress(page transition, 1000, 1000, [suspense page])', ]); await resolveText('Page Two'); @@ -417,6 +441,7 @@ describe('ReactInteractionTracing', () => { await advanceTimers(1000); expect(Scheduler).toFlushAndYield([ 'Page Two', + 'onTransitionProgress(page transition, 1000, 2000, [])', 'onTransitionComplete(page transition, 1000, 2000)', ]); @@ -426,6 +451,7 @@ describe('ReactInteractionTracing', () => { 'Show Text Loading...', 'Page Two', 'onTransitionStart(text transition, 2000)', + 'onTransitionProgress(text transition, 2000, 2000, [show text])', ]); await resolveText('Show Text'); @@ -433,6 +459,7 @@ describe('ReactInteractionTracing', () => { await advanceTimers(1000); expect(Scheduler).toFlushAndYield([ 'Show Text', + 'onTransitionProgress(text transition, 2000, 3000, [])', 'onTransitionComplete(text transition, 2000, 3000)', ]); }); @@ -446,6 +473,12 @@ describe('ReactInteractionTracing', () => { `onTransitionStart(${name}, ${startTime})`, ); }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, onTransitionComplete: (name, startTime, endTime) => { Scheduler.unstable_yieldValue( `onTransitionComplete(${name}, ${startTime}, ${endTime})`, @@ -470,13 +503,15 @@ describe('ReactInteractionTracing', () => { {navigate ? ( <> {showText ? ( - }> + }> ) : null} } - name="suspense page"> + unstable_name="suspense page"> @@ -505,6 +540,7 @@ describe('ReactInteractionTracing', () => { 'Suspend [Page Two]', 'Loading...', 'onTransitionStart(page transition, 1000)', + 'onTransitionProgress(page transition, 1000, 2000, [suspense page])', ]); }); @@ -517,6 +553,7 @@ describe('ReactInteractionTracing', () => { 'Suspend [Page Two]', 'Loading...', 'onTransitionStart(show text, 2000)', + 'onTransitionProgress(show text, 2000, 2000, [show text])', ]); }); @@ -527,6 +564,7 @@ describe('ReactInteractionTracing', () => { expect(Scheduler).toFlushAndYield([ 'Page Two', + 'onTransitionProgress(page transition, 1000, 3000, [])', 'onTransitionComplete(page transition, 1000, 3000)', ]); @@ -536,11 +574,292 @@ describe('ReactInteractionTracing', () => { expect(Scheduler).toFlushAndYield([ 'Show Text', + 'onTransitionProgress(show text, 2000, 4000, [])', 'onTransitionComplete(show text, 2000, 4000)', ]); }); }); + // @gate enableTransitionTracing + it('trace interaction with nested and sibling suspense boundaries', async () => { + const transitionCallbacks = { + onTransitionStart: (name, startTime) => { + Scheduler.unstable_yieldValue( + `onTransitionStart(${name}, ${startTime})`, + ); + }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, + onTransitionComplete: (name, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onTransitionComplete(${name}, ${startTime}, ${endTime})`, + ); + }, + }; + + let navigateToPageTwo; + function App() { + const [navigate, setNavigate] = useState(false); + navigateToPageTwo = () => { + setNavigate(true); + }; + + return ( +
+ {navigate ? ( + <> + } + unstable_name="suspense page"> + + }> + + +
+ }> + + +
+ + + ) : ( + + )} +
+ ); + } + + const root = ReactNoop.createRoot({transitionCallbacks}); + await act(async () => { + root.render(); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield(['Page One']); + }); + + await act(async () => { + startTransition(() => navigateToPageTwo(), {name: 'page transition'}); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Suspend [Page Two]', + 'Suspend [Show Text One]', + 'Show Text One Loading...', + 'Suspend [Show Text Two]', + 'Show Text Two Loading...', + 'Loading...', + 'onTransitionStart(page transition, 1000)', + 'onTransitionProgress(page transition, 1000, 2000, [suspense page])', + ]); + + resolveText('Page Two'); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Page Two', + 'Suspend [Show Text One]', + 'Show Text One Loading...', + 'Suspend [Show Text Two]', + 'Show Text Two Loading...', + 'onTransitionProgress(page transition, 1000, 3000, [show text one, show text two])', + ]); + + resolveText('Show Text One'); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Show Text One', + 'onTransitionProgress(page transition, 1000, 4000, [show text two])', + ]); + + resolveText('Show Text Two'); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Show Text Two', + 'onTransitionProgress(page transition, 1000, 5000, [])', + 'onTransitionComplete(page transition, 1000, 5000)', + ]); + }); + }); + + // @gate enableTransitionTracing + it('trace interactions with the same child suspense boundaries', async () => { + const transitionCallbacks = { + onTransitionStart: (name, startTime) => { + Scheduler.unstable_yieldValue( + `onTransitionStart(${name}, ${startTime})`, + ); + }, + onTransitionProgress: (name, startTime, endTime, pending) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onTransitionProgress(${name}, ${startTime}, ${endTime}, [${suspenseNames}])`, + ); + }, + onTransitionComplete: (name, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onTransitionComplete(${name}, ${startTime}, ${endTime})`, + ); + }, + }; + + let setNavigate; + let setShowTextOne; + let setShowTextTwo; + function App() { + const [navigate, _setNavigate] = useState(false); + const [showTextOne, _setShowTextOne] = useState(false); + const [showTextTwo, _setShowTextTwo] = useState(false); + + setNavigate = () => _setNavigate(true); + setShowTextOne = () => _setShowTextOne(true); + setShowTextTwo = () => _setShowTextTwo(true); + + return ( +
+ {navigate ? ( + <> + } + unstable_name="suspense page"> + + {/* showTextOne is entangled with navigate */} + {showTextOne ? ( + }> + + + ) : null} + }> + + + {/* showTextTwo's suspense boundaries shouldn't stop navigate's suspense boundaries + from completing */} + {showTextTwo ? ( + }> + + + ) : null} + + + ) : ( + + )} +
+ ); + } + + const root = ReactNoop.createRoot({transitionCallbacks}); + await act(async () => { + root.render(); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield(['Page One']); + }); + + await act(async () => { + startTransition(() => setNavigate(), {name: 'navigate'}); + startTransition(() => setShowTextOne(), {name: 'show text one'}); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Suspend [Page Two]', + 'Suspend [Show Text One]', + 'Show Text One Loading...', + 'Suspend [Show Text]', + 'Show Text Loading...', + 'Loading...', + 'onTransitionStart(navigate, 1000)', + 'onTransitionStart(show text one, 1000)', + 'onTransitionProgress(navigate, 1000, 2000, [suspense page])', + 'onTransitionProgress(show text one, 1000, 2000, [suspense page])', + ]); + + resolveText('Page Two'); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ + 'Page Two', + 'Suspend [Show Text One]', + 'Show Text One Loading...', + 'Suspend [Show Text]', + 'Show Text Loading...', + 'onTransitionProgress(navigate, 1000, 3000, [show text one, ])', + 'onTransitionProgress(show text one, 1000, 3000, [show text one, ])', + ]); + + startTransition(() => setShowTextTwo(), {name: 'show text two'}); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Page Two', + 'Suspend [Show Text One]', + 'Show Text One Loading...', + 'Suspend [Show Text]', + 'Show Text Loading...', + 'Suspend [Show Text Two]', + 'Show Text Two Loading...', + 'onTransitionStart(show text two, 3000)', + 'onTransitionProgress(show text two, 3000, 4000, [show text two])', + ]); + + // This should not cause navigate to finish because it's entangled with + // show text one + resolveText('Show Text'); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Show Text', + 'onTransitionProgress(navigate, 1000, 5000, [show text one])', + 'onTransitionProgress(show text one, 1000, 5000, [show text one])', + ]); + + // This should not cause show text two to finish but nothing else + resolveText('Show Text Two'); + ReactNoop.expire(1000); + await advanceTimers(1000); + expect(Scheduler).toFlushAndYield([ + 'Show Text Two', + 'onTransitionProgress(show text two, 3000, 6000, [])', + 'onTransitionComplete(show text two, 3000, 6000)', + ]); + + // This should cause everything to finish + resolveText('Show Text One'); + ReactNoop.expire(1000); + await advanceTimers(1000); + + expect(Scheduler).toFlushAndYield([ + 'Show Text One', + 'onTransitionProgress(navigate, 1000, 7000, [])', + 'onTransitionProgress(show text one, 1000, 7000, [])', + 'onTransitionComplete(navigate, 1000, 7000)', + 'onTransitionComplete(show text one, 1000, 7000)', + ]); + }); + }); + // @gate enableTransitionTracing it('should correctly trace interactions for tracing markers complete', async () => { const transitionCallbacks = {