@samitouri / QOS-React-2 / commits / dd2d652275

[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.

Luna Ruan committed Jul 8, 2022 at 12:31 UTC dd2d6522754f52c70d02c51db25eb7cbd5d1c8eb
5 files changed +116 -14
packages/react-reconciler/src/ReactFiberBeginWork.new.js
+9
@@ -981,6 +981,15 @@ function updateTracingMarkerComponent(
981 };
982 workInProgress.stateNode = markerInstance;
983 }
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 + }
993 }
994
995 const instance: TracingMarkerInstance | null = workInProgress.stateNode;
packages/react-reconciler/src/ReactFiberBeginWork.old.js
+9
@@ -981,6 +981,15 @@ function updateTracingMarkerComponent(
981 };
982 workInProgress.stateNode = markerInstance;
983 }
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 + }
993 }
994
995 const instance: TracingMarkerInstance | null = workInProgress.stateNode;
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+9 -7
@@ -3044,14 +3044,16 @@ function commitPassiveMountOnFiber(
3044 instance.pendingSuspenseBoundaries === null ||
3045 instance.pendingSuspenseBoundaries.size === 0
3046 ) {
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 + });
3053 });
3052 - });
3053 - instance.transitions = null;
3054 - instance.pendingSuspenseBoundaries = null;
3054 + instance.transitions = null;
3055 + instance.pendingSuspenseBoundaries = null;
3056 + }
3057 }
3058 }
3059 break;
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+9 -7
@@ -3044,14 +3044,16 @@ function commitPassiveMountOnFiber(
3044 instance.pendingSuspenseBoundaries === null ||
3045 instance.pendingSuspenseBoundaries.size === 0
3046 ) {
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 + });
3053 });
3052 - });
3053 - instance.transitions = null;
3054 - instance.pendingSuspenseBoundaries = null;
3054 + instance.transitions = null;
3055 + instance.pendingSuspenseBoundaries = null;
3056 + }
3057 }
3058 }
3059 break;
packages/react-reconciler/src/__tests__/ReactTransitionTracing-test.js
+80
@@ -1059,6 +1059,86 @@ describe('ReactInteractionTracing', () => {
1059 });
1060 });
1061
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 +
1142 // @gate enableTransitionTracing
1143 it.skip('marker interaction cancelled when name changes', async () => {
1144 const transitionCallbacks = {