From e225fa43ada4f4cf3d3ba4982cdd81bb093eaa46 Mon Sep 17 00:00:00 2001 From: Luna Ruan Date: Mon, 11 Jul 2022 18:00:49 -0400 Subject: [PATCH] [Transition Tracing] Don't call transition callbacks if no transition name specified (#24887) This PR checks to see if `transition.name` is defined before adding the transition so we avoid doing unnecessary work for transitions without a transition name --- .../src/ReactFiberWorkLoop.new.js | 2 +- .../src/ReactFiberWorkLoop.old.js | 2 +- .../__tests__/ReactTransitionTracing-test.js | 81 +++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 11d41148abb8..67e84f263c08 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -641,7 +641,7 @@ export function scheduleUpdateOnFiber( if (enableTransitionTracing) { const transition = ReactCurrentBatchConfig.transition; - if (transition !== null) { + if (transition !== null && transition.name != null) { if (transition.startTime === -1) { transition.startTime = now(); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 539d7e2f7731..a3a9c88d842b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -641,7 +641,7 @@ export function scheduleUpdateOnFiber( if (enableTransitionTracing) { const transition = ReactCurrentBatchConfig.transition; - if (transition !== null) { + if (transition !== null && transition.name != null) { if (transition.startTime === -1) { transition.startTime = now(); } diff --git a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js index 29d9cc8f492a..bd92a41d4832 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js @@ -160,6 +160,87 @@ describe('ReactInteractionTracing', () => { return Promise.resolve().then(() => {}); } + // @gate enableTransitionTracing + it(' should not call callbacks when transition is not defined', 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})`, + ); + }, + onMarkerProgress: ( + transitioName, + markerName, + startTime, + currentTime, + pending, + ) => { + const suspenseNames = pending.map(p => p.name || '').join(', '); + Scheduler.unstable_yieldValue( + `onMarkerProgress(${transitioName}, ${markerName}, ${startTime}, ${currentTime}, [${suspenseNames}])`, + ); + }, + onMarkerComplete: (transitioName, markerName, startTime, endTime) => { + Scheduler.unstable_yieldValue( + `onMarkerComplete(${transitioName}, ${markerName}, ${startTime}, ${endTime})`, + ); + }, + }; + + function App({navigate}) { + return ( +
+ {navigate ? ( + + + + ) : ( + + )} +
+ ); + } + + 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(() => root.render()); + + ReactNoop.expire(1000); + await advanceTimers(1000); + + // Doesn't call transition or marker code + expect(Scheduler).toFlushAndYield(['Page Two']); + + startTransition(() => root.render(), { + name: 'transition', + }); + expect(Scheduler).toFlushAndYield([ + 'Page One', + 'onTransitionStart(transition, 2000)', + 'onTransitionComplete(transition, 2000, 2000)', + ]); + }); + }); + }); + // @gate enableTransitionTracing it('should correctly trace basic interaction', async () => { const transitionCallbacks = {