[DevTools] Fix Bugs With Component Stacks (#24815)

This PR: * Simplifies the code in `SidebarEventInfo` by passing it the actual clicked event rather than an index. * Lightly refactored the `SidebarEventInfo` code so that it can be used for more than just `schedulingEvents` * Fixes bug. Previously, whenever a state update event was clicked, we updated the `selectedCommitIndex` in the `ProfilerContext`. However, this index is used for the selected commit in the Flamegraph profiler, which caused a bug where if you would change the contents of the event sidebar, the commit sidebar in the Flamegraph profiler would change too. This PR replaces this with the actual event info instead
facebook · Jun 29, 2022 · 1974d08 · 1974d08
1 parent cd80d32
commit 1974d08
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 70 deletions.
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarEventInfo.js b/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarEventInfo.js
@@ -7,80 +7,73 @@
  * @flow
  */
 
+import type {SchedulingEvent} from 'react-devtools-timeline/src/types';
+
 import * as React from 'react';
-import {isStateUpdateEvent} from 'react-devtools-timeline/src/utils/flow';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
 import ViewSourceContext from '../Components/ViewSourceContext';
-import {useContext, useMemo} from 'react';
-import {ProfilerContext} from './ProfilerContext';
+import {useContext} from 'react';
+import {TimelineContext} from 'react-devtools-timeline/src/TimelineContext';
 import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
 
 import styles from './SidebarEventInfo.css';
 
 export type Props = {||};
 
-export default function SidebarEventInfo(_: Props) {
-  const {profilingData, selectedCommitIndex} = useContext(ProfilerContext);
+function SchedulingEventInfo({eventInfo}: {eventInfo: SchedulingEvent}) {
   const {viewUrlSourceFunction} = useContext(ViewSourceContext);
 
-  const {stack} = useMemo(() => {
-    if (
-      selectedCommitIndex == null ||
-      profilingData == null ||
-      profilingData.timelineData.length === 0
-    ) {
-      return {};
-    }
-    const {schedulingEvents} = profilingData.timelineData[0];
+  const componentStack = eventInfo.componentStack
+    ? stackToComponentSources(eventInfo.componentStack)
+    : null;
 
-    const event = schedulingEvents[selectedCommitIndex];
-    if (!isStateUpdateEvent(event)) {
-      return {};
+  const viewSource = source => {
+    if (viewUrlSourceFunction != null && source != null) {
+      viewUrlSourceFunction(...source);
     }
+  };
 
-    let componentStack = null;
-    if (event.componentStack) {
-      componentStack = stackToComponentSources(event.componentStack);
-    }
-
-    return {
-      stack: componentStack,
-    };
-  }, [profilingData, selectedCommitIndex]);
-
-  let components;
-  if (stack) {
-    components = stack.map(([displayName, source], index) => {
-      const hasSource = source != null;
-
-      const onClick = () => {
-        if (viewUrlSourceFunction != null && source != null) {
-          viewUrlSourceFunction(...source);
-        }
-      };
+  return (
+    <div className={styles.Content} tabIndex={0}>
+      {componentStack ? (
+        <ol className={styles.List}>
+          {componentStack.map(([displayName, source], index) => {
+            const hasSource = source != null;
 
-      return (
-        <li key={index} className={styles.ListItem} data-source={hasSource}>
-          <label className={styles.Label}>
-            <Button className={styles.Button} onClick={onClick}>
-              {displayName}
-            </Button>
-            {hasSource && (
-              <ButtonIcon className={styles.Source} type="view-source" />
-            )}
-          </label>
-        </li>
-      );
-    });
-  }
+            return (
+              <li
+                key={index}
+                className={styles.ListItem}
+                data-source={hasSource}>
+                <label className={styles.Label}>
+                  <Button
+                    className={styles.Button}
+                    onClick={() => viewSource(source)}>
+                    {displayName}
+                  </Button>
+                  {hasSource && (
+                    <ButtonIcon className={styles.Source} type="view-source" />
+                  )}
+                </label>
+              </li>
+            );
+          })}
+        </ol>
+      ) : null}
+    </div>
+  );
+}
 
-  return (
+export default function SidebarEventInfo(_: Props) {
+  const {selectedEvent} = useContext(TimelineContext);
+  // (TODO) Refactor in next PR so this supports multiple types of events
+  return selectedEvent ? (
     <>
       <div className={styles.Toolbar}>Event Component Tree</div>
-      <div className={styles.Content} tabIndex={0}>
-        <ol className={styles.List}>{components}</ol>
-      </div>
+      {selectedEvent.schedulingEvent ? (
+        <SchedulingEventInfo eventInfo={selectedEvent.schedulingEvent} />
+      ) : null}
     </>
-  );
+  ) : null;
 }
diff --git a/packages/react-devtools-timeline/src/CanvasPage.js b/packages/react-devtools-timeline/src/CanvasPage.js
@@ -9,7 +9,7 @@
 
 import type {Point} from './view-base';
 import type {
-  ReactHoverContextInfo,
+  ReactEventInfo,
   TimelineData,
   ReactMeasure,
   ViewState,
@@ -63,7 +63,7 @@ import useContextMenu from 'react-devtools-shared/src/devtools/ContextMenu/useCo
 import {getBatchRange} from './utils/getBatchRange';
 import {MAX_ZOOM_LEVEL, MIN_ZOOM_LEVEL} from './view-base/constants';
 import {TimelineSearchContext} from './TimelineSearchContext';
-import {ProfilerContext} from 'react-devtools-shared/src/devtools/views/Profiler/ProfilerContext';
+import {TimelineContext} from './TimelineContext';
 
 import styles from './CanvasPage.css';
 
@@ -132,7 +132,7 @@ const zoomToBatch = (
   viewState.updateHorizontalScrollState(scrollState);
 };
 
-const EMPTY_CONTEXT_INFO: ReactHoverContextInfo = {
+const EMPTY_CONTEXT_INFO: ReactEventInfo = {
   componentMeasure: null,
   flamechartStackFrame: null,
   measure: null,
@@ -162,10 +162,7 @@ function AutoSizedCanvas({
 
   const [isContextMenuShown, setIsContextMenuShown] = useState<boolean>(false);
   const [mouseLocation, setMouseLocation] = useState<Point>(zeroPoint); // DOM coordinates
-  const [
-    hoveredEvent,
-    setHoveredEvent,
-  ] = useState<ReactHoverContextInfo | null>(null);
+  const [hoveredEvent, setHoveredEvent] = useState<ReactEventInfo | null>(null);
 
   const resetHoveredEvent = useCallback(
     () => setHoveredEvent(EMPTY_CONTEXT_INFO),
@@ -529,7 +526,7 @@ function AutoSizedCanvas({
     ref: canvasRef,
   });
 
-  const {selectCommitIndex} = useContext(ProfilerContext);
+  const {selectEvent} = useContext(TimelineContext);
 
   useEffect(() => {
     const {current: userTimingMarksView} = userTimingMarksViewRef;
@@ -566,8 +563,11 @@ function AutoSizedCanvas({
           });
         }
       };
-      schedulingEventsView.onClick = (schedulingEvent, eventIndex) => {
-        selectCommitIndex(eventIndex);
+      schedulingEventsView.onClick = schedulingEvent => {
+        selectEvent({
+          ...EMPTY_CONTEXT_INFO,
+          schedulingEvent,
+        });
       };
     }
 

diff --git a/packages/react-devtools-timeline/src/EventTooltip.js b/packages/react-devtools-timeline/src/EventTooltip.js
@@ -13,7 +13,7 @@ import type {
   NativeEvent,
   NetworkMeasure,
   ReactComponentMeasure,
-  ReactHoverContextInfo,
+  ReactEventInfo,
   ReactMeasure,
   TimelineData,
   SchedulingEvent,
@@ -35,7 +35,7 @@ type Props = {|
   canvasRef: {|current: HTMLCanvasElement | null|},
   data: TimelineData,
   height: number,
-  hoveredEvent: ReactHoverContextInfo | null,
+  hoveredEvent: ReactEventInfo | null,
   origin: Point,
   width: number,
 |};

diff --git a/packages/react-devtools-timeline/src/TimelineContext.js b/packages/react-devtools-timeline/src/TimelineContext.js
@@ -23,6 +23,7 @@ import type {
   TimelineData,
   SearchRegExpStateChangeCallback,
   ViewState,
+  ReactEventInfo,
 } from './types';
 import type {RefObject} from 'shared/ReactTypes';
 
@@ -33,6 +34,8 @@ export type Context = {|
   searchInputContainerRef: RefObject,
   setFile: (file: File | null) => void,
   viewState: ViewState,
+  selectEvent: ReactEventInfo => void,
+  selectedEvent: ReactEventInfo,
 |};
 
 const TimelineContext = createContext<Context>(((null: any): Context));
@@ -121,6 +124,8 @@ function TimelineContextController({children}: Props) {
     return state;
   }, [file]);
 
+  const [selectedEvent, selectEvent] = useState<ReactEventInfo | null>(null);
+
   const value = useMemo(
     () => ({
       file,
@@ -129,8 +134,18 @@ function TimelineContextController({children}: Props) {
       searchInputContainerRef,
       setFile,
       viewState,
+      selectEvent,
+      selectedEvent,
     }),
-    [file, inMemoryTimelineData, isTimelineSupported, setFile, viewState],
+    [
+      file,
+      inMemoryTimelineData,
+      isTimelineSupported,
+      setFile,
+      viewState,
+      selectEvent,
+      selectedEvent,
+    ],
   );
 
   return (

diff --git a/packages/react-devtools-timeline/src/types.js b/packages/react-devtools-timeline/src/types.js
@@ -240,7 +240,7 @@ export type TimelineDataExport = {|
   thrownErrors: ThrownError[],
 |};
 
-export type ReactHoverContextInfo = {|
+export type ReactEventInfo = {|
   componentMeasure: ReactComponentMeasure | null,
   flamechartStackFrame: FlamechartStackFrame | null,
   measure: ReactMeasure | null,