Display warnings in tooltips for native events that render sync updat…

…es (#21975)
facebook · Jul 27, 2021 · cdccdbe · cdccdbe
1 parent d954340
commit cdccdbe
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 16 deletions.
diff --git a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js
@@ -509,6 +509,7 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
       </ContextMenu>
       {!isContextMenuShown && !surfaceRef.current.hasActiveView() && (
         <EventTooltip
+          canvasRef={canvasRef}
           data={data}
           hoveredEvent={hoveredEvent}
           origin={mouseLocation}

diff --git a/packages/react-devtools-scheduling-profiler/src/EventTooltip.js b/packages/react-devtools-scheduling-profiler/src/EventTooltip.js
@@ -28,6 +28,7 @@ import useSmartTooltip from './utils/useSmartTooltip';
 import styles from './EventTooltip.css';
 
 type Props = {|
+  canvasRef: {|current: HTMLCanvasElement | null|},
   data: ReactProfilerData,
   hoveredEvent: ReactHoverContextInfo | null,
   origin: Point,
@@ -102,8 +103,14 @@ function getReactMeasureLabel(type): string | null {
   }
 }
 
-export default function EventTooltip({data, hoveredEvent, origin}: Props) {
+export default function EventTooltip({
+  canvasRef,
+  data,
+  hoveredEvent,
+  origin,
+}: Props) {
   const tooltipRef = useSmartTooltip({
+    canvasRef,
     mouseX: origin.x,
     mouseY: origin.y,
   });
@@ -209,7 +216,19 @@ const TooltipNativeEvent = ({
   nativeEvent: NativeEvent,
   tooltipRef: Return<typeof useRef>,
 }) => {
-  const {duration, timestamp, type} = nativeEvent;
+  const {duration, timestamp, type, warnings} = nativeEvent;
+
+  const warningElements = [];
+  if (warnings !== null) {
+    warnings.forEach((warning, index) => {
+      warningElements.push(
+        <Fragment key={index}>
+          <div className={styles.DetailsGridLabel}>Warning:</div>
+          <div>{warning}</div>
+        </Fragment>,
+      );
+    });
+  }
 
   return (
     <div className={styles.Tooltip} ref={tooltipRef}>
@@ -221,6 +240,7 @@ const TooltipNativeEvent = ({
         <div>{formatTimestamp(timestamp)}</div>
         <div className={styles.DetailsGridLabel}>Duration:</div>
         <div>{formatDuration(duration)}</div>
+        {warningElements}
       </div>
     </div>
   );

diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/NativeEventsView.js b/packages/react-devtools-scheduling-profiler/src/content-views/NativeEventsView.js
@@ -128,7 +128,7 @@ export class NativeEventsView extends View {
     showHoverHighlight: boolean,
   ) {
     const {frame} = this;
-    const {depth, duration, highlight, timestamp, type} = event;
+    const {depth, duration, timestamp, type, warnings} = event;
 
     baseY += depth * ROW_WITH_BORDER_HEIGHT;
 
@@ -152,7 +152,7 @@ export class NativeEventsView extends View {
 
     const drawableRect = intersectionOfRects(eventRect, rect);
     context.beginPath();
-    if (highlight) {
+    if (warnings !== null) {
       context.fillStyle = showHoverHighlight
         ? COLORS.NATIVE_EVENT_WARNING_HOVER
         : COLORS.NATIVE_EVENT_WARNING;
@@ -182,9 +182,10 @@ export class NativeEventsView extends View {
       );
 
       if (trimmedName !== null) {
-        context.fillStyle = highlight
-          ? COLORS.NATIVE_EVENT_WARNING_TEXT
-          : COLORS.TEXT_COLOR;
+        context.fillStyle =
+          warnings !== null
+            ? COLORS.NATIVE_EVENT_WARNING_TEXT
+            : COLORS.TEXT_COLOR;
 
         context.fillText(
           trimmedName,

diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js
@@ -202,9 +202,9 @@ function processTimelineEvent(
         const nativeEvent = {
           depth,
           duration,
-          highlight: false,
           timestamp,
           type,
+          warnings: null,
         };
 
         currentProfilerData.nativeEvents.push(nativeEvent);
@@ -339,8 +339,13 @@ function processTimelineEvent(
           const nativeEvent = nativeEventStack[i];
           const stopTime = nativeEvent.timestamp + nativeEvent.duration;
           if (stopTime > startTime) {
-            // Warn about sync updates that happen an event handler.
-            nativeEvent.highlight = true;
+            const warning =
+              'An event handler scheduled a synchronous update with React.';
+            if (nativeEvent.warnings === null) {
+              nativeEvent.warnings = new Set([warning]);
+            } else {
+              nativeEvent.warnings.add(warning);
+            }
           }
         }
       } else if (

diff --git a/packages/react-devtools-scheduling-profiler/src/types.js b/packages/react-devtools-scheduling-profiler/src/types.js
@@ -24,9 +24,9 @@ export type ReactLane = number;
 export type NativeEvent = {|
   +depth: number,
   +duration: Milliseconds,
-  highlight: boolean,
   +timestamp: Milliseconds,
   +type: string,
+  warnings: Set<string> | null,
 |};
 
 type BaseReactEvent = {|

diff --git a/packages/react-devtools-scheduling-profiler/src/utils/useSmartTooltip.js b/packages/react-devtools-scheduling-profiler/src/utils/useSmartTooltip.js
@@ -12,22 +12,32 @@ import {useLayoutEffect, useRef} from 'react';
 const TOOLTIP_OFFSET = 4;
 
 export default function useSmartTooltip({
+  canvasRef,
   mouseX,
   mouseY,
 }: {
+  canvasRef: {|current: HTMLCanvasElement | null|},
   mouseX: number,
   mouseY: number,
 }) {
   const ref = useRef<HTMLElement | null>(null);
 
+  // HACK: Browser extension reports window.innerHeight of 0,
+  // so we fallback to using the tooltip target element.
+  let height = window.innerHeight;
+  let width = window.innerWidth;
+  const target = canvasRef.current;
+  if (target !== null) {
+    const rect = target.getBoundingClientRect();
+    height = rect.top + rect.height;
+    width = rect.left + rect.width;
+  }
+
   useLayoutEffect(() => {
     const element = ref.current;
     if (element !== null) {
       // Let's check the vertical position.
-      if (
-        mouseY + TOOLTIP_OFFSET + element.offsetHeight >=
-        window.innerHeight
-      ) {
+      if (mouseY + TOOLTIP_OFFSET + element.offsetHeight >= height) {
         // The tooltip doesn't fit below the mouse cursor (which is our
         // default strategy). Therefore we try to position it either above the
         // mouse cursor or finally aligned with the window's top edge.
@@ -45,7 +55,7 @@ export default function useSmartTooltip({
       }
 
       // Now let's check the horizontal position.
-      if (mouseX + TOOLTIP_OFFSET + element.offsetWidth >= window.innerWidth) {
+      if (mouseX + TOOLTIP_OFFSET + element.offsetWidth >= width) {
         // The tooltip doesn't fit at the right of the mouse cursor (which is
         // our default strategy). Therefore we try to position it either at the
         // left of the mouse cursor or finally aligned with the window's left