Skip to content

Commit

Permalink
[Transition Tracing] Add Tag Field to Marker Instance (#25085)
Browse files Browse the repository at this point in the history
We were previously using `markerInstance.name` to figure out whether the marker instance was on the tracing marker or the root, but this is unsustainable. This adds a tag field so we can explicitly check this.
  • Loading branch information
lunaruan authored and rickhanlonii committed Oct 6, 2022
1 parent 8fc85dd commit b6b0f30
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 8 deletions.
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ import {
REACT_CACHE_TYPE,
REACT_TRACING_MARKER_TYPE,
} from 'shared/ReactSymbols';
import {TransitionTracingMarker} from './ReactFiberTracingMarkerComponent.new';

export type {Fiber};

Expand Down Expand Up @@ -770,6 +771,7 @@ export function createFiberFromTracingMarker(
fiber.elementType = REACT_TRACING_MARKER_TYPE;
fiber.lanes = lanes;
const tracingMarkerInstance: TracingMarkerInstance = {
tag: TransitionTracingMarker,
transitions: null,
pendingBoundaries: null,
};
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiber.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ import {
REACT_CACHE_TYPE,
REACT_TRACING_MARKER_TYPE,
} from 'shared/ReactSymbols';
import {TransitionTracingMarker} from './ReactFiberTracingMarkerComponent.old';

export type {Fiber};

Expand Down Expand Up @@ -770,6 +771,7 @@ export function createFiberFromTracingMarker(
fiber.elementType = REACT_TRACING_MARKER_TYPE;
fiber.lanes = lanes;
const tracingMarkerInstance: TracingMarkerInstance = {
tag: TransitionTracingMarker,
transitions: null,
pendingBoundaries: null,
};
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ import {
getMarkerInstances,
pushMarkerInstance,
pushRootMarkerInstance,
TransitionTracingMarker,
} from './ReactFiberTracingMarkerComponent.new';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
Expand Down Expand Up @@ -976,6 +977,7 @@ function updateTracingMarkerComponent(
const currentTransitions = getPendingTransitions();
if (currentTransitions !== null) {
const markerInstance: TracingMarkerInstance = {
tag: TransitionTracingMarker,
transitions: new Set(currentTransitions),
pendingBoundaries: new Map(),
name: workInProgress.pendingProps.name,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ import {
getMarkerInstances,
pushMarkerInstance,
pushRootMarkerInstance,
TransitionTracingMarker,
} from './ReactFiberTracingMarkerComponent.old';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
Expand Down Expand Up @@ -976,6 +977,7 @@ function updateTracingMarkerComponent(
const currentTransitions = getPendingTransitions();
if (currentTransitions !== null) {
const markerInstance: TracingMarkerInstance = {
tag: TransitionTracingMarker,
transitions: new Set(currentTransitions),
pendingBoundaries: new Map(),
name: workInProgress.pendingProps.name,
Expand Down
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ import {
OffscreenVisible,
OffscreenPassiveEffectsConnected,
} from './ReactFiberOffscreenComponent';
import {
TransitionRoot,
TransitionTracingMarker,
} from './ReactFiberTracingMarkerComponent.new';

let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
if (__DEV__) {
Expand Down Expand Up @@ -1184,13 +1188,16 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
name,
});
if (transitions !== null) {
if (markerInstance.name) {
if (
markerInstance.tag === TransitionTracingMarker &&
markerInstance.name !== undefined
) {
addMarkerProgressCallbackToPendingTransition(
markerInstance.name,
transitions,
pendingBoundaries,
);
} else {
} else if (markerInstance.tag === TransitionRoot) {
transitions.forEach(transition => {
addTransitionProgressCallbackToPendingTransition(
transition,
Expand All @@ -1216,13 +1223,16 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
) {
pendingBoundaries.delete(offscreenInstance);
if (transitions !== null) {
if (markerInstance.name) {
if (
markerInstance.tag === TransitionTracingMarker &&
markerInstance.name !== undefined
) {
addMarkerProgressCallbackToPendingTransition(
markerInstance.name,
transitions,
pendingBoundaries,
);
} else {
} else if (markerInstance.tag === TransitionRoot) {
transitions.forEach(transition => {
addTransitionProgressCallbackToPendingTransition(
transition,
Expand Down
18 changes: 14 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ import {
OffscreenVisible,
OffscreenPassiveEffectsConnected,
} from './ReactFiberOffscreenComponent';
import {
TransitionRoot,
TransitionTracingMarker,
} from './ReactFiberTracingMarkerComponent.old';

let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
if (__DEV__) {
Expand Down Expand Up @@ -1184,13 +1188,16 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
name,
});
if (transitions !== null) {
if (markerInstance.name) {
if (
markerInstance.tag === TransitionTracingMarker &&
markerInstance.name !== undefined
) {
addMarkerProgressCallbackToPendingTransition(
markerInstance.name,
transitions,
pendingBoundaries,
);
} else {
} else if (markerInstance.tag === TransitionRoot) {
transitions.forEach(transition => {
addTransitionProgressCallbackToPendingTransition(
transition,
Expand All @@ -1216,13 +1223,16 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
) {
pendingBoundaries.delete(offscreenInstance);
if (transitions !== null) {
if (markerInstance.name) {
if (
markerInstance.tag === TransitionTracingMarker &&
markerInstance.name !== undefined
) {
addMarkerProgressCallbackToPendingTransition(
markerInstance.name,
transitions,
pendingBoundaries,
);
} else {
} else if (markerInstance.tag === TransitionRoot) {
transitions.forEach(transition => {
addTransitionProgressCallbackToPendingTransition(
transition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,16 @@ export type BatchConfigTransition = {
};

export type TracingMarkerInstance = {|
tag?: TracingMarkerTag,
pendingBoundaries: PendingBoundaries | null,
transitions: Set<Transition> | null,
name?: string,
|};

export const TransitionRoot = 0;
export const TransitionTracingMarker = 1;
export type TracingMarkerTag = 0 | 1;

export type PendingBoundaries = Map<OffscreenInstance, SuspenseInfo>;

export function processTransitionCallbacks(
Expand Down Expand Up @@ -146,6 +151,7 @@ export function pushRootMarkerInstance(workInProgress: Fiber): void {
transitions.forEach(transition => {
if (!root.incompleteTransitions.has(transition)) {
const markerInstance: TracingMarkerInstance = {
tag: TransitionRoot,
transitions: new Set([transition]),
pendingBoundaries: null,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,16 @@ export type BatchConfigTransition = {
};

export type TracingMarkerInstance = {|
tag?: TracingMarkerTag,
pendingBoundaries: PendingBoundaries | null,
transitions: Set<Transition> | null,
name?: string,
|};

export const TransitionRoot = 0;
export const TransitionTracingMarker = 1;
export type TracingMarkerTag = 0 | 1;

export type PendingBoundaries = Map<OffscreenInstance, SuspenseInfo>;

export function processTransitionCallbacks(
Expand Down Expand Up @@ -146,6 +151,7 @@ export function pushRootMarkerInstance(workInProgress: Fiber): void {
transitions.forEach(transition => {
if (!root.incompleteTransitions.has(transition)) {
const markerInstance: TracingMarkerInstance = {
tag: TransitionRoot,
transitions: new Set([transition]),
pendingBoundaries: null,
};
Expand Down

0 comments on commit b6b0f30

Please sign in to comment.