Skip to content

Commit dd2d652

Browse files
authored
[Transition Tracing] Tracing Marker Name Change in Update Warning (#24873)
We should only support Tracing Marker's name field during component mount. This PR adds a warning if the Tracing Marker's name changes during an update.
1 parent 80208e7 commit dd2d652

5 files changed

+116
-14
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.new.js

+9
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,15 @@ function updateTracingMarkerComponent(
981981
};
982982
workInProgress.stateNode = markerInstance;
983983
}
984+
} else {
985+
if (__DEV__) {
986+
if (current.memoizedProps.name !== workInProgress.pendingProps.name) {
987+
console.error(
988+
'Changing the name of a tracing marker after mount is not supported. ' +
989+
'To remount the tracing marker, pass it a new key.',
990+
);
991+
}
992+
}
984993
}
985994

986995
const instance: TracingMarkerInstance | null = workInProgress.stateNode;

packages/react-reconciler/src/ReactFiberBeginWork.old.js

+9
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,15 @@ function updateTracingMarkerComponent(
981981
};
982982
workInProgress.stateNode = markerInstance;
983983
}
984+
} else {
985+
if (__DEV__) {
986+
if (current.memoizedProps.name !== workInProgress.pendingProps.name) {
987+
console.error(
988+
'Changing the name of a tracing marker after mount is not supported. ' +
989+
'To remount the tracing marker, pass it a new key.',
990+
);
991+
}
992+
}
984993
}
985994

986995
const instance: TracingMarkerInstance | null = workInProgress.stateNode;

packages/react-reconciler/src/ReactFiberCommitWork.new.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -3044,14 +3044,16 @@ function commitPassiveMountOnFiber(
30443044
instance.pendingSuspenseBoundaries === null ||
30453045
instance.pendingSuspenseBoundaries.size === 0
30463046
) {
3047-
instance.transitions.forEach(transition => {
3048-
addMarkerCompleteCallbackToPendingTransition({
3049-
transition,
3050-
name: finishedWork.memoizedProps.name,
3047+
if (instance.transitions !== null) {
3048+
instance.transitions.forEach(transition => {
3049+
addMarkerCompleteCallbackToPendingTransition({
3050+
transition,
3051+
name: finishedWork.memoizedProps.name,
3052+
});
30513053
});
3052-
});
3053-
instance.transitions = null;
3054-
instance.pendingSuspenseBoundaries = null;
3054+
instance.transitions = null;
3055+
instance.pendingSuspenseBoundaries = null;
3056+
}
30553057
}
30563058
}
30573059
break;

packages/react-reconciler/src/ReactFiberCommitWork.old.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -3044,14 +3044,16 @@ function commitPassiveMountOnFiber(
30443044
instance.pendingSuspenseBoundaries === null ||
30453045
instance.pendingSuspenseBoundaries.size === 0
30463046
) {
3047-
instance.transitions.forEach(transition => {
3048-
addMarkerCompleteCallbackToPendingTransition({
3049-
transition,
3050-
name: finishedWork.memoizedProps.name,
3047+
if (instance.transitions !== null) {
3048+
instance.transitions.forEach(transition => {
3049+
addMarkerCompleteCallbackToPendingTransition({
3050+
transition,
3051+
name: finishedWork.memoizedProps.name,
3052+
});
30513053
});
3052-
});
3053-
instance.transitions = null;
3054-
instance.pendingSuspenseBoundaries = null;
3054+
instance.transitions = null;
3055+
instance.pendingSuspenseBoundaries = null;
3056+
}
30553057
}
30563058
}
30573059
break;

packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js

+80
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,86 @@ describe('ReactInteractionTracing', () => {
10591059
});
10601060
});
10611061

1062+
// @gate enableTransitionTracing
1063+
it('warns when marker name changes', async () => {
1064+
const transitionCallbacks = {
1065+
onTransitionStart: (name, startTime) => {
1066+
Scheduler.unstable_yieldValue(
1067+
`onTransitionStart(${name}, ${startTime})`,
1068+
);
1069+
},
1070+
onTransitionComplete: (name, startTime, endTime) => {
1071+
Scheduler.unstable_yieldValue(
1072+
`onTransitionComplete(${name}, ${startTime}, ${endTime})`,
1073+
);
1074+
},
1075+
onMarkerComplete: (transitioName, markerName, startTime, endTime) => {
1076+
Scheduler.unstable_yieldValue(
1077+
`onMarkerComplete(${transitioName}, ${markerName}, ${startTime}, ${endTime})`,
1078+
);
1079+
},
1080+
};
1081+
function App({markerName, markerKey}) {
1082+
return (
1083+
<React.unstable_TracingMarker name={markerName} key={markerKey}>
1084+
<Text text={markerName} />
1085+
</React.unstable_TracingMarker>
1086+
);
1087+
}
1088+
1089+
const root = ReactNoop.createRoot({transitionCallbacks});
1090+
await act(async () => {
1091+
startTransition(
1092+
() => root.render(<App markerName="one" markerKey="key" />),
1093+
{
1094+
name: 'transition one',
1095+
},
1096+
);
1097+
ReactNoop.expire(1000);
1098+
await advanceTimers(1000);
1099+
expect(Scheduler).toFlushAndYield([
1100+
'one',
1101+
'onTransitionStart(transition one, 0)',
1102+
'onMarkerComplete(transition one, one, 0, 1000)',
1103+
'onTransitionComplete(transition one, 0, 1000)',
1104+
]);
1105+
startTransition(
1106+
() => root.render(<App markerName="two" markerKey="key" />),
1107+
{
1108+
name: 'transition two',
1109+
},
1110+
);
1111+
ReactNoop.expire(1000);
1112+
await advanceTimers(1000);
1113+
expect(() => {
1114+
// onMarkerComplete shouldn't be called for transitions with
1115+
// new keys
1116+
expect(Scheduler).toFlushAndYield([
1117+
'two',
1118+
'onTransitionStart(transition two, 1000)',
1119+
'onTransitionComplete(transition two, 1000, 2000)',
1120+
]);
1121+
}).toErrorDev(
1122+
'Changing the name of a tracing marker after mount is not supported.',
1123+
);
1124+
startTransition(
1125+
() => root.render(<App markerName="three" markerKey="new key" />),
1126+
{
1127+
name: 'transition three',
1128+
},
1129+
);
1130+
ReactNoop.expire(1000);
1131+
await advanceTimers(1000);
1132+
// This should not warn and onMarkerComplete should be called
1133+
expect(Scheduler).toFlushAndYield([
1134+
'three',
1135+
'onTransitionStart(transition three, 2000)',
1136+
'onMarkerComplete(transition three, three, 2000, 3000)',
1137+
'onTransitionComplete(transition three, 2000, 3000)',
1138+
]);
1139+
});
1140+
});
1141+
10621142
// @gate enableTransitionTracing
10631143
it.skip('marker interaction cancelled when name changes', async () => {
10641144
const transitionCallbacks = {

0 commit comments

Comments
 (0)