[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
facebook · Jul 11, 2022 · e225fa4 · e225fa4
1 parent dd2d652
commit e225fa4
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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 || '<null>').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 || '<null>').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 (
+        <div>
+          {navigate ? (
+            <React.unstable_TracingMarker name="marker">
+              <Text text="Page Two" />
+            </React.unstable_TracingMarker>
+          ) : (
+            <Text text="Page One" />
+          )}
+        </div>
+      );
+    }
+
+    const root = ReactNoop.createRoot({transitionCallbacks});
+    await act(async () => {
+      root.render(<App navigate={false} />);
+      ReactNoop.expire(1000);
+      await advanceTimers(1000);
+
+      expect(Scheduler).toFlushAndYield(['Page One']);
+
+      await act(async () => {
+        startTransition(() => root.render(<App navigate={true} />));
+
+        ReactNoop.expire(1000);
+        await advanceTimers(1000);
+
+        // Doesn't call transition or marker code
+        expect(Scheduler).toFlushAndYield(['Page Two']);
+
+        startTransition(() => root.render(<App navigate={false} />), {
+          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 = {