Scheduling Profiler should not warn about long updates when they are …

…transition priority. This is the purpose of the transition lane and APIs.
facebook · Oct 21, 2021 · 639dd59 · 639dd59
1 parent b81de86
commit 639dd59
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 1 deletion.
diff --git a/...-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js b/...-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js
@@ -1581,6 +1581,142 @@ describe('preprocessData', () => {
           );
         }
       });
+
+      it('should not warn about transition updates scheduled during commit phase', async () => {
+        function Component() {
+          const [value, setValue] = React.useState(0);
+          const [isPending, startTransition] = React.useTransition();
+
+          Scheduler.unstable_yieldValue(
+            `Component rendered with value ${value}`,
+          );
+
+          // Fake a long render
+          if (value !== 0) {
+            Scheduler.unstable_yieldValue('Long render');
+            startTime += 20000;
+          }
+
+          React.useLayoutEffect(() => {
+            startTransition(() => {
+              setValue(1);
+            });
+          }, []);
+
+          return value;
+        }
+
+        if (gate(flags => flags.enableSchedulingProfiler)) {
+          const cpuProfilerSample = creactCpuProfilerSample();
+
+          const root = ReactDOM.createRoot(document.createElement('div'));
+          act(() => {
+            root.render(<Component />);
+          });
+
+          expect(Scheduler).toHaveYielded([
+            'Component rendered with value 0',
+            'Component rendered with value 0',
+            'Component rendered with value 1',
+            'Long render',
+          ]);
+
+          const testMarks = [];
+          clearedMarks.forEach(markName => {
+            if (markName === '--component-render-start-Component') {
+              // Fake a long running render
+              startTime += 20000;
+            }
+
+            testMarks.push({
+              pid: ++pid,
+              tid: ++tid,
+              ts: ++startTime,
+              args: {data: {}},
+              cat: 'blink.user_timing',
+              name: markName,
+              ph: 'R',
+            });
+          });
+
+          const data = await preprocessData([
+            cpuProfilerSample,
+            ...createBoilerplateEntries(),
+            ...testMarks,
+          ]);
+
+          data.schedulingEvents.forEach(event => {
+            expect(event.warning).toBeNull();
+          });
+        }
+      });
+
+      it('should not warn about deferred value updates scheduled during commit phase', async () => {
+        function Component() {
+          const [value, setValue] = React.useState(0);
+          const deferredValue = React.useDeferredValue(value);
+
+          Scheduler.unstable_yieldValue(
+            `Component rendered with value ${value} and deferredValue ${deferredValue}`,
+          );
+
+          // Fake a long render
+          if (deferredValue !== 0) {
+            Scheduler.unstable_yieldValue('Long render');
+            startTime += 20000;
+          }
+
+          React.useLayoutEffect(() => {
+            setValue(1);
+          }, []);
+
+          return value + deferredValue;
+        }
+
+        if (gate(flags => flags.enableSchedulingProfiler)) {
+          const cpuProfilerSample = creactCpuProfilerSample();
+
+          const root = ReactDOM.createRoot(document.createElement('div'));
+          act(() => {
+            root.render(<Component />);
+          });
+
+          expect(Scheduler).toHaveYielded([
+            'Component rendered with value 0 and deferredValue 0',
+            'Component rendered with value 1 and deferredValue 0',
+            'Component rendered with value 1 and deferredValue 1',
+            'Long render',
+          ]);
+
+          const testMarks = [];
+          clearedMarks.forEach(markName => {
+            if (markName === '--component-render-start-Component') {
+              // Fake a long running render
+              startTime += 20000;
+            }
+
+            testMarks.push({
+              pid: ++pid,
+              tid: ++tid,
+              ts: ++startTime,
+              args: {data: {}},
+              cat: 'blink.user_timing',
+              name: markName,
+              ph: 'R',
+            });
+          });
+
+          const data = await preprocessData([
+            cpuProfilerSample,
+            ...createBoilerplateEntries(),
+            ...testMarks,
+          ]);
+
+          data.schedulingEvents.forEach(event => {
+            expect(event.warning).toBeNull();
+          });
+        }
+      });
     });
 
     describe('errors thrown while rendering', () => {

diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js
@@ -970,7 +970,16 @@ export default async function preprocessData(
     // See how long the subsequent batch of React work was.
     const [startTime, stopTime] = getBatchRange(batchUID, profilerData);
     if (stopTime - startTime > NESTED_UPDATE_DURATION_THRESHOLD) {
-      schedulingEvent.warning = WARNING_STRINGS.NESTED_UPDATE;
+      // Don't warn about transition updates scheduled during the commit phase.
+      // e.g. useTransition, useDeferredValue
+      // These are allowed to be long-running.
+      if (
+        !schedulingEvent.lanes.some(
+          lane => profilerData.laneToLabelMap.get(lane) === 'Transition',
+        )
+      ) {
+        schedulingEvent.warning = WARNING_STRINGS.NESTED_UPDATE;
+      }
     }
   });
   state.potentialSuspenseEventsOutsideOfTransition.forEach(