Unify BatchConfigTransition and Transition types (#32783)

This is some overdue refactoring. The two types never made sense. It
also should be defined by isomorphic since it defines how it should be
used by renderers rather than isomorphic depending on Fiber.

Clean up hidden classes to be consistent.

Fix missing name due to wrong types. I choose not to invoke the
transition tracing callbacks if there's no name since the name is
required there.

Author: sebmarkbage

packages/react-reconciler/src/ReactFiberActivityComponent.js

@@ -10,10 +10,8 @@
 import type {ReactNodeList, OffscreenMode, Wakeable} from 'shared/ReactTypes';
 import type {Lanes} from './ReactFiberLane';
 import type {SpawnedCachePool} from './ReactFiberCacheComponent';
-import type {
-  Transition,
-  TracingMarkerInstance,
-} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
+import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent';
 import type {RetryQueue} from './ReactFiberSuspenseComponent';
 
 export type OffscreenProps = {

packages/react-reconciler/src/ReactFiberAsyncAction.js

@@ -13,7 +13,7 @@ import type {
   RejectedThenable,
 } from 'shared/ReactTypes';
 import type {Lane} from './ReactFiberLane';
-import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
 
 import {requestTransitionLane} from './ReactFiberRootScheduler';
 import {NoLane} from './ReactFiberLane';
@@ -46,7 +46,7 @@ let currentEntangledLane: Lane = NoLane;
 let currentEntangledActionThenable: Thenable<void> | null = null;
 
 export function entangleAsyncAction<S>(
-  transition: BatchConfigTransition,
+  transition: Transition,
   thenable: Thenable<S>,
 ): Thenable<S> {
   // `thenable` is the return value of the async action scope function. Create

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -30,8 +30,8 @@ import type {
 } from './ReactFiberActivityComponent';
 import type {Cache} from './ReactFiberCacheComponent';
 import type {RootState} from './ReactFiberRoot';
+import type {Transition} from 'react/src/ReactStartTransition';
 import type {
-  Transition,
   TracingMarkerInstance,
   TransitionAbort,
 } from './ReactFiberTracingMarkerComponent';

packages/react-reconciler/src/ReactFiberHooks.js

@@ -149,7 +149,7 @@ import {
   SuspenseActionException,
 } from './ReactFiberThenable';
 import type {ThenableState} from './ReactFiberThenable';
-import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
 import {
   peekEntangledActionLane,
   peekEntangledActionThenable,
@@ -2137,11 +2137,15 @@ function runActionStateAction<S, P>(
 
     // This is a fork of startTransition
     const prevTransition = ReactSharedInternals.T;
-    const currentTransition: BatchConfigTransition = {};
-    ReactSharedInternals.T = currentTransition;
+    const currentTransition: Transition = ({}: any);
+    if (enableTransitionTracing) {
+      currentTransition.name = null;
+      currentTransition.startTime = -1;
+    }
     if (__DEV__) {
-      ReactSharedInternals.T._updatedFibers = new Set();
+      currentTransition._updatedFibers = new Set();
     }
+    ReactSharedInternals.T = currentTransition;
     try {
       const returnValue = action(prevState, payload);
       const onStartTransitionFinish = ReactSharedInternals.S;
@@ -3012,7 +3016,15 @@ function startTransition<S>(
   );
 
   const prevTransition = ReactSharedInternals.T;
-  const currentTransition: BatchConfigTransition = {};
+  const currentTransition: Transition = ({}: any);
+  if (enableTransitionTracing) {
+    currentTransition.name =
+      options !== undefined && options.name !== undefined ? options.name : null;
+    currentTransition.startTime = now();
+  }
+  if (__DEV__) {
+    currentTransition._updatedFibers = new Set();
+  }
 
   // We don't really need to use an optimistic update here, because we
   // schedule a second "revert" update below (which we use to suspend the
@@ -3023,17 +3035,6 @@ function startTransition<S>(
   ReactSharedInternals.T = currentTransition;
   dispatchOptimisticSetState(fiber, false, queue, pendingState);
 
-  if (enableTransitionTracing) {
-    if (options !== undefined && options.name !== undefined) {
-      currentTransition.name = options.name;
-      currentTransition.startTime = now();
-    }
-  }
-
-  if (__DEV__) {
-    currentTransition._updatedFibers = new Set();
-  }
-
   try {
     const returnValue = callback();
     const onStartTransitionFinish = ReactSharedInternals.S;

packages/react-reconciler/src/ReactFiberLane.js

@@ -8,7 +8,7 @@
  */
 
 import type {Fiber, FiberRoot} from './ReactInternalTypes';
-import type {Transition} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
 import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates';
 
 // TODO: Ideally these types would be opaque but that doesn't work well with

packages/react-reconciler/src/ReactFiberRootScheduler.js

@@ -10,7 +10,7 @@
 import type {FiberRoot} from './ReactInternalTypes';
 import type {Lane, Lanes} from './ReactFiberLane';
 import type {PriorityLevel} from 'scheduler/src/SchedulerPriorities';
-import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
 
 import {
   disableLegacyMode,
@@ -635,7 +635,7 @@ export function requestTransitionLane(
   // This argument isn't used, it's only here to encourage the caller to
   // check that it's inside a transition before calling this function.
   // TODO: Make this non-nullable. Requires a tweak to useOptimistic.
-  transition: BatchConfigTransition | null,
+  transition: Transition | null,
 ): Lane {
   // The algorithm for assigning an update to a lane should be stable for all
   // updates at the same priority within the same event. To do this, the

packages/react-reconciler/src/ReactFiberTracingMarkerComponent.js

@@ -12,6 +12,7 @@ import type {
   Fiber,
   FiberRoot,
 } from './ReactInternalTypes';
+import type {Transition} from 'react/src/ReactStartTransition';
 import type {OffscreenInstance} from './ReactFiberActivityComponent';
 import type {StackCursor} from './ReactFiberStack';
 
@@ -36,19 +37,6 @@ export type PendingTransitionCallbacks = {
   markerComplete: Map<string, Set<Transition>> | null,
 };
 
-// TODO: Unclear to me why these are separate types
-export type Transition = {
-  name: string,
-  startTime: number,
-  ...
-};
-
-export type BatchConfigTransition = {
-  name?: string,
-  startTime?: number,
-  _updatedFibers?: Set<Fiber>,
-};
-
 // TODO: Is there a way to not include the tag or name here?
 export type TracingMarkerInstance = {
   tag?: TracingMarkerTag,
@@ -79,9 +67,11 @@ export function processTransitionCallbacks(
       const transitionStart = pendingTransitions.transitionStart;
       const onTransitionStart = callbacks.onTransitionStart;
       if (transitionStart !== null && onTransitionStart != null) {
-        transitionStart.forEach(transition =>
-          onTransitionStart(transition.name, transition.startTime),
-        );
+        transitionStart.forEach(transition => {
+          if (transition.name != null) {
+            onTransitionStart(transition.name, transition.startTime);
+          }
+        });
       }
 
       const markerProgress = pendingTransitions.markerProgress;
@@ -95,13 +85,15 @@ export function processTransitionCallbacks(
                 ? Array.from(markerInstance.pendingBoundaries.values())
                 : [];
             markerInstance.transitions.forEach(transition => {
-              onMarkerProgress(
-                transition.name,
-                markerName,
-                transition.startTime,
-                endTime,
-                pending,
-              );
+              if (transition.name != null) {
+                onMarkerProgress(
+                  transition.name,
+                  markerName,
+                  transition.startTime,
+                  endTime,
+                  pending,
+                );
+              }
             });
           }
         });
@@ -112,12 +104,14 @@ export function processTransitionCallbacks(
       if (markerComplete !== null && onMarkerComplete != null) {
         markerComplete.forEach((transitions, markerName) => {
           transitions.forEach(transition => {
-            onMarkerComplete(
-              transition.name,
-              markerName,
-              transition.startTime,
-              endTime,
-            );
+            if (transition.name != null) {
+              onMarkerComplete(
+                transition.name,
+                markerName,
+                transition.startTime,
+                endTime,
+              );
+            }
           });
         });
       }
@@ -153,12 +147,14 @@ export function processTransitionCallbacks(
             });
 
             if (filteredAborts.length > 0) {
-              onMarkerIncomplete(
-                transition.name,
-                markerName,
-                transition.startTime,
-                filteredAborts,
-              );
+              if (transition.name != null) {
+                onMarkerIncomplete(
+                  transition.name,
+                  markerName,
+                  transition.startTime,
+                  filteredAborts,
+                );
+              }
             }
           });
         });
@@ -168,21 +164,29 @@ export function processTransitionCallbacks(
       const onTransitionProgress = callbacks.onTransitionProgress;
       if (onTransitionProgress != null && transitionProgress !== null) {
         transitionProgress.forEach((pending, transition) => {
-          onTransitionProgress(
-            transition.name,
-            transition.startTime,
-            endTime,
-            Array.from(pending.values()),
-          );
+          if (transition.name != null) {
+            onTransitionProgress(
+              transition.name,
+              transition.startTime,
+              endTime,
+              Array.from(pending.values()),
+            );
+          }
         });
       }
 
       const transitionComplete = pendingTransitions.transitionComplete;
       const onTransitionComplete = callbacks.onTransitionComplete;
       if (transitionComplete !== null && onTransitionComplete != null) {
-        transitionComplete.forEach(transition =>
-          onTransitionComplete(transition.name, transition.startTime, endTime),
-        );
+        transitionComplete.forEach(transition => {
+          if (transition.name != null) {
+            onTransitionComplete(
+              transition.name,
+              transition.startTime,
+              endTime,
+            );
+          }
+        });
       }
     }
   }

packages/react-reconciler/src/ReactFiberTransition.js

@@ -11,10 +11,7 @@ import type {Thenable} from 'shared/ReactTypes';
 import type {Lanes} from './ReactFiberLane';
 import type {StackCursor} from './ReactFiberStack';
 import type {Cache, SpawnedCachePool} from './ReactFiberCacheComponent';
-import type {
-  BatchConfigTransition,
-  Transition,
-} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
 
 import {enableTransitionTracing} from 'shared/ReactFeatureFlags';
 import {isPrimaryRenderer} from './ReactFiberConfig';
@@ -57,7 +54,7 @@ export const NoTransition = null;
 // reconciler. Leaving this for a future PR.
 const prevOnStartTransitionFinish = ReactSharedInternals.S;
 ReactSharedInternals.S = function onStartTransitionFinishForReconciler(
-  transition: BatchConfigTransition,
+  transition: Transition,
   returnValue: mixed,
 ) {
   if (
@@ -81,7 +78,7 @@ ReactSharedInternals.S = function onStartTransitionFinishForReconciler(
   }
 };
 
-export function requestCurrentTransition(): BatchConfigTransition | null {
+export function requestCurrentTransition(): Transition | null {
   return ReactSharedInternals.T;
 }
 

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -14,10 +14,10 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes';
 import type {Lanes, Lane} from './ReactFiberLane';
 import type {SuspenseState} from './ReactFiberSuspenseComponent';
 import type {FunctionComponentUpdateQueue} from './ReactFiberHooks';
+import type {Transition} from 'react/src/ReactStartTransition';
 import type {
   PendingTransitionCallbacks,
   PendingBoundaries,
-  Transition,
   TransitionAbort,
 } from './ReactFiberTracingMarkerComponent';
 import type {OffscreenInstance} from './ReactFiberActivityComponent';
@@ -927,8 +927,6 @@ export function scheduleUpdateOnFiber(
           transition.startTime = now();
         }
 
-        // $FlowFixMe[prop-missing]: The BatchConfigTransition and Transition types are incompatible but was previously untyped and thus uncaught
-        // $FlowFixMe[incompatible-call]: "
         addTransitionToLanesMap(root, transition, lane);
       }
     }

packages/react-reconciler/src/ReactInternalTypes.js

@@ -33,10 +33,8 @@ import type {
   TransitionStatus,
 } from './ReactFiberConfig';
 import type {Cache} from './ReactFiberCacheComponent';
-import type {
-  TracingMarkerInstance,
-  Transition,
-} from './ReactFiberTracingMarkerComponent';
+import type {Transition} from 'react/src/ReactStartTransition';
+import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent';
 import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates';
 import type {ComponentStackNode} from 'react-server/src/ReactFizzComponentStack';
 import type {ThenableState} from './ReactFiberThenable';

packages/react/src/ReactSharedInternalsClient.js

@@ -9,14 +9,14 @@
 
 import type {Dispatcher} from 'react-reconciler/src/ReactInternalTypes';
 import type {AsyncDispatcher} from 'react-reconciler/src/ReactInternalTypes';
-import type {BatchConfigTransition} from 'react-reconciler/src/ReactFiberTracingMarkerComponent';
+import type {Transition} from './ReactStartTransition';
 import type {TransitionTypes} from './ReactTransitionType';
 
 export type SharedStateClient = {
   H: null | Dispatcher, // ReactCurrentDispatcher for Hooks
   A: null | AsyncDispatcher, // ReactCurrentCache for Cache
-  T: null | BatchConfigTransition, // ReactCurrentBatchConfig for Transitions
-  S: null | ((BatchConfigTransition, mixed) => void), // onStartTransitionFinish
+  T: null | Transition, // ReactCurrentBatchConfig for Transitions
+  S: null | ((Transition, mixed) => void), // onStartTransitionFinish
   V: null | TransitionTypes, // Pending Transition Types for the Next Transition
 
   // DEV-only