@samitouri / QOS-React-2 / commits / c4c51c0821

Prevent filter preference changes while profiling is active

Brian Vaughn committed Apr 30, 2019 at 09:48 UTC c4c51c0821d6ac37513da12d9174d0fe47a1390e
3 files changed +28 -1
src/backend/renderer.js
+7
@@ -273,6 +273,12 @@ export function attach(
273
274 // TODO (filter) We could make this more efficient.
275 function updateFilterPreferences(filterPreferences: FilterPreferences) {
276 + if (this._isProfiling) {
277 + // Re-mounting a tree while profiling is in progress might break a lot of assumptions.
278 + // If necessary, we could support this- but it doesn't seem like a necessary use case.
279 + throw Error('Cannot modify filter preferences while profiling');
280 + }
281 +
282 // Recursively unmount and then re-mount all roots.
283 hook.getFiberRoots(rendererID).forEach(root => {
284 currentRootID = getFiberID(getPrimaryFiber(root.current));
@@ -2218,6 +2224,7 @@ export function attach(
2224 rootPseudoKeys.delete(id);
2225 }
2226
2227 + // TODO (profiling) This breaks after filter preferences have been updated.
2228 function getPathFrame(fiber: Fiber): PathFrame {
2229 let { displayName, key } = getDataForFiber(fiber);
2230 const index = fiber.index;
src/devtools/store.js
+6 -1
@@ -243,12 +243,17 @@ export default class Store extends EventEmitter {
243 return this._filterPreferences;
244 }
245 set filterPreferences(value: FilterPreferences): void {
246 + if (this._isProfiling) {
247 + // Re-mounting a tree while profiling is in progress might break a lot of assumptions.
248 + // If necessary, we could support this- but it doesn't seem like a necessary use case.
249 + throw Error('Cannot modify filter preferences while profiling');
250 + }
251 +
252 this._filterPreferences = value;
253
254 saveFilterPreferences(value);
255
256 // TODO (filter) Dump all nodes, update renderer preferences, and re-initialize tree.
251 - // TODO (filter) Invariant check that we aren't profiling.
257 // TODO (filter) Flushing every time a filter setting is changed is too expensive. We probably need an explitit configm
258 this._bridge.send('updateFilterPreferences', value);
259
src/devtools/views/Settings/Settings.js
+15
@@ -46,6 +46,20 @@ function Settings(_: {||}) {
46 collapseNodesByDefaultSubscription
47 );
48
49 + // Re-mounting a tree while profiling is in progress might break a lot of assumptions.
50 + // If necessary, we could support this- but it doesn't seem like a necessary use case.
51 + const isProfilingSubscription = useMemo(
52 + () => ({
53 + getCurrentValue: () => store.isProfiling,
54 + subscribe: (callback: Function) => {
55 + store.addListener('isProfiling', callback);
56 + return () => store.removeListener('isProfiling', callback);
57 + },
58 + }),
59 + [store]
60 + );
61 + const isProfiling = useSubscription<boolean, Store>(isProfilingSubscription);
62 +
63 const filterPreferencesSubscription = useMemo(
64 () => ({
65 getCurrentValue: () => store.filterPreferences,
@@ -182,6 +196,7 @@ function Settings(_: {||}) {
196 checked={filterPreferences.hideElementsWithTypes.has(
197 ElementTypeHostComponent
198 )}
199 + disabled={isProfiling}
200 onChange={updateFilterPreferences}
201 />{' '}
202 Hide host components (e.g. <code>&lt;div&gt;</code>)