@samitouri / QOS-React-2 / commits / 1974d08c93

[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

Luna Ruan committed Jun 29, 2022 at 19:00 UTC 1974d08c93ddf171779fde59215e8430f36d281a
5 files changed +78 -70
packages/react-devtools-shared/src/devtools/views/Profiler/SidebarEventInfo.js
+49 -56
@@ -7,80 +7,73 @@
7 * @flow
8 */
9
10 +import type {SchedulingEvent} from 'react-devtools-timeline/src/types';
11 +
12 import * as React from 'react';
11 -import {isStateUpdateEvent} from 'react-devtools-timeline/src/utils/flow';
13 import Button from '../Button';
14 import ButtonIcon from '../ButtonIcon';
15 import ViewSourceContext from '../Components/ViewSourceContext';
15 -import {useContext, useMemo} from 'react';
16 -import {ProfilerContext} from './ProfilerContext';
16 +import {useContext} from 'react';
17 +import {TimelineContext} from 'react-devtools-timeline/src/TimelineContext';
18 import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
19
20 import styles from './SidebarEventInfo.css';
21
22 export type Props = {||};
23
23 -export default function SidebarEventInfo(_: Props) {
24 - const {profilingData, selectedCommitIndex} = useContext(ProfilerContext);
24 +function SchedulingEventInfo({eventInfo}: {eventInfo: SchedulingEvent}) {
25 const {viewUrlSourceFunction} = useContext(ViewSourceContext);
26
27 - const {stack} = useMemo(() => {
28 - if (
29 - selectedCommitIndex == null ||
30 - profilingData == null ||
31 - profilingData.timelineData.length === 0
32 - ) {
33 - return {};
34 - }
35 - const {schedulingEvents} = profilingData.timelineData[0];
27 + const componentStack = eventInfo.componentStack
28 + ? stackToComponentSources(eventInfo.componentStack)
29 + : null;
30
37 - const event = schedulingEvents[selectedCommitIndex];
38 - if (!isStateUpdateEvent(event)) {
39 - return {};
31 + const viewSource = source => {
32 + if (viewUrlSourceFunction != null && source != null) {
33 + viewUrlSourceFunction(...source);
34 }
35 + };
36
42 - let componentStack = null;
43 - if (event.componentStack) {
44 - componentStack = stackToComponentSources(event.componentStack);
45 - }
46 -
47 - return {
48 - stack: componentStack,
49 - };
50 - }, [profilingData, selectedCommitIndex]);
51 -
52 - let components;
53 - if (stack) {
54 - components = stack.map(([displayName, source], index) => {
55 - const hasSource = source != null;
56 -
57 - const onClick = () => {
58 - if (viewUrlSourceFunction != null && source != null) {
59 - viewUrlSourceFunction(...source);
60 - }
61 - };
37 + return (
38 + <div className={styles.Content} tabIndex={0}>
39 + {componentStack ? (
40 + <ol className={styles.List}>
41 + {componentStack.map(([displayName, source], index) => {
42 + const hasSource = source != null;
43
63 - return (
64 - <li key={index} className={styles.ListItem} data-source={hasSource}>
65 - <label className={styles.Label}>
66 - <Button className={styles.Button} onClick={onClick}>
67 - {displayName}
68 - </Button>
69 - {hasSource && (
70 - <ButtonIcon className={styles.Source} type="view-source" />
71 - )}
72 - </label>
73 - </li>
74 - );
75 - });
76 - }
44 + return (
45 + <li
46 + key={index}
47 + className={styles.ListItem}
48 + data-source={hasSource}>
49 + <label className={styles.Label}>
50 + <Button
51 + className={styles.Button}
52 + onClick={() => viewSource(source)}>
53 + {displayName}
54 + </Button>
55 + {hasSource && (
56 + <ButtonIcon className={styles.Source} type="view-source" />
57 + )}
58 + </label>
59 + </li>
60 + );
61 + })}
62 + </ol>
63 + ) : null}
64 + </div>
65 + );
66 +}
67
78 - return (
68 +export default function SidebarEventInfo(_: Props) {
69 + const {selectedEvent} = useContext(TimelineContext);
70 + // (TODO) Refactor in next PR so this supports multiple types of events
71 + return selectedEvent ? (
72 <>
73 <div className={styles.Toolbar}>Event Component Tree</div>
81 - <div className={styles.Content} tabIndex={0}>
82 - <ol className={styles.List}>{components}</ol>
83 - </div>
74 + {selectedEvent.schedulingEvent ? (
75 + <SchedulingEventInfo eventInfo={selectedEvent.schedulingEvent} />
76 + ) : null}
77 </>
85 - );
78 + ) : null;
79 }
packages/react-devtools-timeline/src/CanvasPage.js
+10 -10
@@ -9,7 +9,7 @@
9
10 import type {Point} from './view-base';
11 import type {
12 - ReactHoverContextInfo,
12 + ReactEventInfo,
13 TimelineData,
14 ReactMeasure,
15 ViewState,
@@ -63,7 +63,7 @@ import useContextMenu from 'react-devtools-shared/src/devtools/ContextMenu/useCo
63 import {getBatchRange} from './utils/getBatchRange';
64 import {MAX_ZOOM_LEVEL, MIN_ZOOM_LEVEL} from './view-base/constants';
65 import {TimelineSearchContext} from './TimelineSearchContext';
66 -import {ProfilerContext} from 'react-devtools-shared/src/devtools/views/Profiler/ProfilerContext';
66 +import {TimelineContext} from './TimelineContext';
67
68 import styles from './CanvasPage.css';
69
@@ -132,7 +132,7 @@ const zoomToBatch = (
132 viewState.updateHorizontalScrollState(scrollState);
133 };
134
135 -const EMPTY_CONTEXT_INFO: ReactHoverContextInfo = {
135 +const EMPTY_CONTEXT_INFO: ReactEventInfo = {
136 componentMeasure: null,
137 flamechartStackFrame: null,
138 measure: null,
@@ -162,10 +162,7 @@ function AutoSizedCanvas({
162
163 const [isContextMenuShown, setIsContextMenuShown] = useState<boolean>(false);
164 const [mouseLocation, setMouseLocation] = useState<Point>(zeroPoint); // DOM coordinates
165 - const [
166 - hoveredEvent,
167 - setHoveredEvent,
168 - ] = useState<ReactHoverContextInfo | null>(null);
165 + const [hoveredEvent, setHoveredEvent] = useState<ReactEventInfo | null>(null);
166
167 const resetHoveredEvent = useCallback(
168 () => setHoveredEvent(EMPTY_CONTEXT_INFO),
@@ -529,7 +526,7 @@ function AutoSizedCanvas({
526 ref: canvasRef,
527 });
528
532 - const {selectCommitIndex} = useContext(ProfilerContext);
529 + const {selectEvent} = useContext(TimelineContext);
530
531 useEffect(() => {
532 const {current: userTimingMarksView} = userTimingMarksViewRef;
@@ -566,8 +563,11 @@ function AutoSizedCanvas({
563 });
564 }
565 };
569 - schedulingEventsView.onClick = (schedulingEvent, eventIndex) => {
570 - selectCommitIndex(eventIndex);
566 + schedulingEventsView.onClick = schedulingEvent => {
567 + selectEvent({
568 + ...EMPTY_CONTEXT_INFO,
569 + schedulingEvent,
570 + });
571 };
572 }
573
packages/react-devtools-timeline/src/EventTooltip.js
+2 -2
@@ -13,7 +13,7 @@ import type {
13 NativeEvent,
14 NetworkMeasure,
15 ReactComponentMeasure,
16 - ReactHoverContextInfo,
16 + ReactEventInfo,
17 ReactMeasure,
18 TimelineData,
19 SchedulingEvent,
@@ -35,7 +35,7 @@ type Props = {|
35 canvasRef: {|current: HTMLCanvasElement | null|},
36 data: TimelineData,
37 height: number,
38 - hoveredEvent: ReactHoverContextInfo | null,
38 + hoveredEvent: ReactEventInfo | null,
39 origin: Point,
40 width: number,
41 |};
packages/react-devtools-timeline/src/TimelineContext.js
+16 -1
@@ -23,6 +23,7 @@ import type {
23 TimelineData,
24 SearchRegExpStateChangeCallback,
25 ViewState,
26 + ReactEventInfo,
27 } from './types';
28 import type {RefObject} from 'shared/ReactTypes';
29
@@ -33,6 +34,8 @@ export type Context = {|
34 searchInputContainerRef: RefObject,
35 setFile: (file: File | null) => void,
36 viewState: ViewState,
37 + selectEvent: ReactEventInfo => void,
38 + selectedEvent: ReactEventInfo,
39 |};
40
41 const TimelineContext = createContext<Context>(((null: any): Context));
@@ -121,6 +124,8 @@ function TimelineContextController({children}: Props) {
124 return state;
125 }, [file]);
126
127 + const [selectedEvent, selectEvent] = useState<ReactEventInfo | null>(null);
128 +
129 const value = useMemo(
130 () => ({
131 file,
@@ -129,8 +134,18 @@ function TimelineContextController({children}: Props) {
134 searchInputContainerRef,
135 setFile,
136 viewState,
137 + selectEvent,
138 + selectedEvent,
139 }),
133 - [file, inMemoryTimelineData, isTimelineSupported, setFile, viewState],
140 + [
141 + file,
142 + inMemoryTimelineData,
143 + isTimelineSupported,
144 + setFile,
145 + viewState,
146 + selectEvent,
147 + selectedEvent,
148 + ],
149 );
150
151 return (
packages/react-devtools-timeline/src/types.js
+1 -1
@@ -240,7 +240,7 @@ export type TimelineDataExport = {|
240 thrownErrors: ThrownError[],
241 |};
242
243 -export type ReactHoverContextInfo = {|
243 +export type ReactEventInfo = {|
244 componentMeasure: ReactComponentMeasure | null,
245 flamechartStackFrame: FlamechartStackFrame | null,
246 measure: ReactMeasure | null,