Profiler bugfix for filtering out all commits after selecting a fiber
Brian Vaughn committed
Aug 18, 2019 at 08:34 UTC
4697f5b37967b85b2c844044aeebb5b1a740875d
4 files changed
+47
-26
src/devtools/views/Profiler/Profiler.js
+13
-4
@@ -28,6 +28,7 @@ function Profiler(_: {||}) {
28
didRecordCommits,
29
isProcessingData,
30
isProfiling,
31
+ selectedCommitIndex,
32
selectedFiberID,
33
selectedTabID,
34
selectTab,
@@ -67,10 +68,18 @@ function Profiler(_: {||}) {
68
break;
69
case 'flame-chart':
70
case 'ranked-chart':
70
- if (selectedFiberID !== null) {
71
- sidebar = <SidebarSelectedFiberInfo />;
72
- } else {
73
- sidebar = <SidebarCommitInfo />;
71
+ // TRICKY
72
+ // Handle edge case where no commit is selected because of a min-duration filter update.
73
+ // In that case, the selected commit index would be null.
74
+ // We could still show a sidebar for the previously selected fiber,
75
+ // but it would be an odd user experience.
76
+ // TODO (ProfilerContext) This check should not be necessary.
77
+ if (selectedCommitIndex !== null) {
78
+ if (selectedFiberID !== null) {
79
+ sidebar = <SidebarSelectedFiberInfo />;
80
+ } else {
81
+ sidebar = <SidebarCommitInfo />;
82
+ }
83
}
84
break;
85
default:
src/devtools/views/Profiler/ProfilerContext.js
+24
-20
@@ -127,28 +127,32 @@ function ProfilerContextController({ children }: Props) {
127
const [rootID, setRootID] = useState<number | null>(null);
128
129
if (prevProfilingData !== profilingData) {
130
- setPrevProfilingData(profilingData);
131
-
132
- const dataForRoots =
133
- profilingData !== null ? profilingData.dataForRoots : null;
134
- if (dataForRoots != null) {
135
- const firstRootID = dataForRoots.keys().next().value || null;
136
-
137
- if (rootID === null || !dataForRoots.has(rootID)) {
138
- let selectedElementRootID = null;
139
- if (selectedElementID !== null) {
140
- selectedElementRootID = store.getRootIDForElement(selectedElementID);
141
- }
142
- if (
143
- selectedElementRootID !== null &&
144
- dataForRoots.has(selectedElementRootID)
145
- ) {
146
- setRootID(selectedElementRootID);
147
- } else {
148
- setRootID(firstRootID);
130
+ batchedUpdates(() => {
131
+ setPrevProfilingData(profilingData);
132
+
133
+ const dataForRoots =
134
+ profilingData !== null ? profilingData.dataForRoots : null;
135
+ if (dataForRoots != null) {
136
+ const firstRootID = dataForRoots.keys().next().value || null;
137
+
138
+ if (rootID === null || !dataForRoots.has(rootID)) {
139
+ let selectedElementRootID = null;
140
+ if (selectedElementID !== null) {
141
+ selectedElementRootID = store.getRootIDForElement(
142
+ selectedElementID
143
+ );
144
+ }
145
+ if (
146
+ selectedElementRootID !== null &&
147
+ dataForRoots.has(selectedElementRootID)
148
+ ) {
149
+ setRootID(selectedElementRootID);
150
+ } else {
151
+ setRootID(firstRootID);
152
+ }
153
}
154
}
151
- }
155
+ });
156
}
157
158
const startProfiling = useCallback(
src/devtools/views/Profiler/SidebarSelectedFiberInfo.js
+9
-1
@@ -89,7 +89,7 @@ export default function SidebarSelectedFiberInfo(_: Props) {
89
}
90
91
type WhatChangedProps = {|
92
- commitIndex: number,
92
+ commitIndex: number | null,
93
fiberID: number,
94
profilerStore: ProfilerStore,
95
rootID: number,
@@ -101,6 +101,14 @@ function WhatChanged({
101
profilerStore,
102
rootID,
103
}: WhatChangedProps) {
104
+ // TRICKY
105
+ // Handle edge case where no commit is selected because of a min-duration filter update.
106
+ // If the commit index is null, suspending for data below would throw an error.
107
+ // TODO (ProfilerContext) This check should not be necessary.
108
+ if (commitIndex === null) {
109
+ return null;
110
+ }
111
+
112
const { changeDescriptions } = profilerStore.getCommitData(
113
((rootID: any): number),
114
commitIndex
src/devtools/views/Profiler/SnapshotSelector.js
+1
-1
@@ -59,7 +59,7 @@ export default function SnapshotSelector(_: Props) {
59
return null;
60
}, [filteredCommitIndices, selectedCommitIndex]);
61
62
- // TODO (profiling) This should be managed by the context controller (reducer).
62
+ // TODO (ProfilerContext) This should be managed by the context controller (reducer).
63
// It doesn't currently know about the filtered commits though (since it doesn't suspend).
64
// Maybe this component should pass filteredCommitIndices up?
65
if (selectedFilteredCommitIndex === null) {