From 99b6a44beb5b10a517c4af93e2e5b5ea07ed86e9 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 22 May 2019 07:40:41 -0700 Subject: [PATCH] Updated event subscriptions and getters to use new ProfilerStore --- src/__tests__/profilerStore-test.js | 4 +- src/__tests__/profilingCache-test.js | 34 +++++----- src/__tests__/profilingCharts-test.js | 48 +++++++------- .../profilingCommitTreeBuilder-test.js | 6 +- src/__tests__/storeComponentFilters-test.js | 6 +- src/devtools/ProfilerStore.js | 33 ++-------- src/devtools/store.js | 63 ------------------- .../views/Components/ComponentFiltersModal.js | 9 +-- .../ToggleComponentFiltersModalButton.js | 9 +-- .../Profiler/ClearProfilingDataButton.js | 7 ++- .../views/Profiler/CommitFlamegraph.js | 3 +- src/devtools/views/Profiler/CommitRanked.js | 3 +- src/devtools/views/Profiler/Interactions.js | 3 +- .../views/Profiler/ProfilerContext.js | 9 ++- .../Profiler/ProfilingImportExportButtons.js | 4 +- .../views/Profiler/SidebarInteractions.js | 3 +- .../Profiler/SidebarSelectedFiberInfo.js | 3 +- 17 files changed, 91 insertions(+), 156 deletions(-) diff --git a/src/__tests__/profilerStore-test.js b/src/__tests__/profilerStore-test.js index d2f7eefef7..08f64b344b 100644 --- a/src/__tests__/profilerStore-test.js +++ b/src/__tests__/profilerStore-test.js @@ -34,14 +34,14 @@ describe('ProfilerStore', () => { ReactDOM.render(, containerB); }); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => { ReactDOM.render(, containerA); ReactDOM.render(, containerB); }); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); const rootA = store.roots[0]; const rootB = store.roots[1]; diff --git a/src/__tests__/profilingCache-test.js b/src/__tests__/profilingCache-test.js index 04e4a9fdda..8ea903dee1 100644 --- a/src/__tests__/profilingCache-test.js +++ b/src/__tests__/profilingCache-test.js @@ -51,11 +51,11 @@ describe('ProfilingCache', () => { const container = document.createElement('div'); utils.act(() => ReactDOM.render(, container)); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let profilingDataForRoot = null; @@ -120,12 +120,12 @@ describe('ProfilingCache', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); const allCommitData = []; @@ -200,11 +200,11 @@ describe('ProfilingCache', () => { return null; }; - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => ReactDOM.render(, document.createElement('div')) ); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let commitData = null; @@ -262,11 +262,11 @@ describe('ProfilingCache', () => { return data; }; - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); await utils.actAsync(() => ReactDOM.render(, document.createElement('div')) ); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); const allCommitData = []; @@ -317,16 +317,16 @@ describe('ProfilingCache', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); const allFiberCommits = []; function Suspender({ fiberID, previousFiberCommits, rootID }) { - const fiberCommits = store.profilingCache.getFiberCommits({ + const fiberCommits = store.profilerStore.profilingCache.getFiberCommits({ fiberID, rootID, }); @@ -407,7 +407,7 @@ describe('ProfilingCache', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => SchedulerTracing.unstable_trace( 'mount: one child', @@ -422,14 +422,16 @@ describe('ProfilingCache', () => { () => ReactDOM.render(, container) ) ); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let interactions = null; function Suspender({ previousInteractions, rootID }) { - interactions = store.profilingCache.getInteractionsChartData({ - rootID, - }).interactions; + interactions = store.profilerStore.profilingCache.getInteractionsChartData( + { + rootID, + } + ).interactions; if (previousInteractions != null) { expect(interactions).toEqual(previousInteractions); } else { diff --git a/src/__tests__/profilingCharts-test.js b/src/__tests__/profilingCharts-test.js index 33d698600b..cebbdc0d54 100644 --- a/src/__tests__/profilingCharts-test.js +++ b/src/__tests__/profilingCharts-test.js @@ -47,7 +47,7 @@ describe('profiling charts', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => SchedulerTracing.unstable_trace('mount', Scheduler.unstable_now(), () => ReactDOM.render(, container) @@ -60,20 +60,22 @@ describe('profiling charts', () => { () => ReactDOM.render(, container) ) ); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let renderFinished = false; function Suspender({ commitIndex, rootID }) { - const commitTree = store.profilingCache.getCommitTree({ + const commitTree = store.profilerStore.profilingCache.getCommitTree({ commitIndex, rootID, }); - const chartData = store.profilingCache.getFlamegraphChartData({ - commitIndex, - commitTree, - rootID, - }); + const chartData = store.profilerStore.profilingCache.getFlamegraphChartData( + { + commitIndex, + commitTree, + rootID, + } + ); expect(commitTree).toMatchSnapshot(`${commitIndex}: CommitTree`); expect(chartData).toMatchSnapshot( `${commitIndex}: FlamegraphChartData` @@ -127,7 +129,7 @@ describe('profiling charts', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => SchedulerTracing.unstable_trace('mount', Scheduler.unstable_now(), () => ReactDOM.render(, container) @@ -140,20 +142,22 @@ describe('profiling charts', () => { () => ReactDOM.render(, container) ) ); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let renderFinished = false; function Suspender({ commitIndex, rootID }) { - const commitTree = store.profilingCache.getCommitTree({ + const commitTree = store.profilerStore.profilingCache.getCommitTree({ commitIndex, rootID, }); - const chartData = store.profilingCache.getRankedChartData({ - commitIndex, - commitTree, - rootID, - }); + const chartData = store.profilerStore.profilingCache.getRankedChartData( + { + commitIndex, + commitTree, + rootID, + } + ); expect(commitTree).toMatchSnapshot(`${commitIndex}: CommitTree`); expect(chartData).toMatchSnapshot(`${commitIndex}: RankedChartData`); renderFinished = true; @@ -203,7 +207,7 @@ describe('profiling charts', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => SchedulerTracing.unstable_trace('mount', Scheduler.unstable_now(), () => ReactDOM.render(, container) @@ -216,14 +220,16 @@ describe('profiling charts', () => { () => ReactDOM.render(, container) ) ); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let renderFinished = false; function Suspender({ commitIndex, rootID }) { - const chartData = store.profilingCache.getInteractionsChartData({ - rootID, - }); + const chartData = store.profilerStore.profilingCache.getInteractionsChartData( + { + rootID, + } + ); expect(chartData).toMatchSnapshot('Interactions'); renderFinished = true; return null; diff --git a/src/__tests__/profilingCommitTreeBuilder-test.js b/src/__tests__/profilingCommitTreeBuilder-test.js index 31d7acafcf..17cf9b9e7b 100644 --- a/src/__tests__/profilingCommitTreeBuilder-test.js +++ b/src/__tests__/profilingCommitTreeBuilder-test.js @@ -38,17 +38,17 @@ describe('commit tree', () => { const container = document.createElement('div'); - utils.act(() => store.startProfiling()); + utils.act(() => store.profilerStore.startProfiling()); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); utils.act(() => ReactDOM.render(, container)); - utils.act(() => store.stopProfiling()); + utils.act(() => store.profilerStore.stopProfiling()); let renderFinished = false; function Suspender({ commitIndex, rootID }) { - const commitTree = store.profilingCache.getCommitTree({ + const commitTree = store.profilerStore.profilingCache.getCommitTree({ commitIndex, rootID, }); diff --git a/src/__tests__/storeComponentFilters-test.js b/src/__tests__/storeComponentFilters-test.js index 3030646bf6..dff169e924 100644 --- a/src/__tests__/storeComponentFilters-test.js +++ b/src/__tests__/storeComponentFilters-test.js @@ -1,11 +1,13 @@ // @flow +import type Store from 'src/devtools/store'; + describe('Store component filters', () => { let React; let ReactDOM; let TestUtils; let Types; - let store; + let store: Store; let utils; const act = (callback: Function) => { @@ -28,7 +30,7 @@ describe('Store component filters', () => { }); it('should throw if filters are updated while profiling', () => { - act(() => store.startProfiling()); + act(() => store.profilerStore.startProfiling()); expect(() => (store.componentFilters = [])).toThrow( 'Cannot modify filter preferences while profiling' ); diff --git a/src/devtools/ProfilerStore.js b/src/devtools/ProfilerStore.js index 41b547682a..9f255fd963 100644 --- a/src/devtools/ProfilerStore.js +++ b/src/devtools/ProfilerStore.js @@ -114,10 +114,6 @@ export default class ProfilerStore extends EventEmitter { throw Error(`Could not find commit data for root "${rootID}"`); } - get cache(): ProfilingCache { - return this._cache; - } - // Profiling data has been recorded for at least one root. get hasProfilingData(): boolean { return ( @@ -125,21 +121,6 @@ export default class ProfilerStore extends EventEmitter { ); } - // TODO (profarc) Remove this getter - get initialSnapshotsByRootID(): Map> { - return this._initialSnapshotsByRootID; - } - - // TODO (profarc) Remove this getter - get inProgressOperationsByRootID(): Map> { - return this._inProgressOperationsByRootID; - } - - // TODO (profarc) Remove this getter - get inProgressScreenshotsByRootID(): Map> { - return this._inProgressScreenshotsByRootID; - } - get isProcessingData(): boolean { return this._rendererQueue.size > 0 || this._dataBackends.length > 0; } @@ -148,6 +129,10 @@ export default class ProfilerStore extends EventEmitter { return this._isProfiling; } + get profilingCache(): ProfilingCache { + return this._cache; + } + get profilingData(): ProfilingDataFrontend | null { return this._dataFrontend; } @@ -159,8 +144,6 @@ export default class ProfilerStore extends EventEmitter { this._inProgressScreenshotsByRootID.clear(); this._cache.invalidate(); - // TODO (profarc) Remove subscriptions to Store for this - this._store.emit('profilingData'); this.emit('profilingData'); } @@ -175,8 +158,6 @@ export default class ProfilerStore extends EventEmitter { // Note that we clear now because any existing data is "stale". this._cache.invalidate(); - // TODO (profarc) Remove subscriptions to Store for this - this._store.emit('isProfiling'); this.emit('isProfiling'); } @@ -282,8 +263,6 @@ export default class ProfilerStore extends EventEmitter { this._dataBackends.splice(0); - // TODO (profarc) Remove subscriptions to Store for this - this._store.emit('isProcessingData'); this.emit('isProcessingData'); } }; @@ -320,8 +299,6 @@ export default class ProfilerStore extends EventEmitter { // (That would have resolved a now-stale value without any profiling data.) this._cache.invalidate(); - // TODO (profarc) Remove subscriptions to Store for this - this._store.emit('isProfiling'); this.emit('isProfiling'); // If we've just finished a profiling session, we need to fetch data stored in each renderer interface @@ -339,8 +316,6 @@ export default class ProfilerStore extends EventEmitter { } } - // TODO (profarc) Remove subscriptions to Store for this - this._store.emit('isProcessingData'); this.emit('isProcessingData'); } } diff --git a/src/devtools/store.js b/src/devtools/store.js index be368ee794..41b4534cc0 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -15,15 +15,10 @@ import { utfDecodeString, } from '../utils'; import { __DEBUG__ } from '../constants'; -import ProfilingCache from './ProfilingCache'; import { printStore } from 'src/__tests__/storeSerializer'; import ProfilerStore from './ProfilerStore'; import type { Element } from './views/Components/types'; -import type { - ProfilingDataFrontend, - SnapshotNode, -} from './views/Profiler/types'; import type { Bridge, ComponentFilter, ElementType } from '../types'; const debug = (methodName, ...args) => { @@ -236,53 +231,10 @@ export default class Store extends EventEmitter { return this._hasOwnerMetadata; } - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get hasProfilingData(): boolean { - return this._profilerStore.hasProfilingData; - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get isProcessingProfilingData(): boolean { - return this._profilerStore.isProcessingData; - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get isProfiling(): boolean { - return this._profilerStore.isProfiling; - } - get numElements(): number { return this._weightAcrossRoots; } - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get profilingCache(): ProfilingCache { - return this._profilerStore.cache; - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get profilingData(): ProfilingDataFrontend | null { - return this._profilerStore.profilingData; - } - set profilingData(value: ProfilingDataFrontend | null): void { - this._profilerStore.profilingData = value; - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get profilingOperationsByRootID(): Map> { - return this._profilerStore.inProgressOperationsByRootID; - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get profilingScreenshotsByRootID(): Map> { - return this._profilerStore.inProgressScreenshotsByRootID; - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this value. - get profilingSnapshotsByRootID(): Map> { - return this._profilerStore.initialSnapshotsByRootID; - } - get profilerStore(): ProfilerStore { return this._profilerStore; } @@ -311,11 +263,6 @@ export default class Store extends EventEmitter { return this._supportsReloadAndProfile; } - // TODO (profarc) Update views to use ProfilerStore directly to access this method. - clearProfilingData(): void { - this._profilerStore.clear(); - } - containsElement(id: number): boolean { return this._idToElement.get(id) != null; } @@ -541,16 +488,6 @@ export default class Store extends EventEmitter { return false; } - // TODO (profarc) Update views to use ProfilerStore directly to access this method. - startProfiling(): void { - this._profilerStore.startProfiling(); - } - - // TODO (profarc) Update views to use ProfilerStore directly to access this method. - stopProfiling(): void { - this._profilerStore.stopProfiling(); - } - // TODO Maybe split this into two methods: expand() and collapse() toggleIsCollapsed(id: number, isCollapsed: boolean): void { let didMutate = false; diff --git a/src/devtools/views/Components/ComponentFiltersModal.js b/src/devtools/views/Components/ComponentFiltersModal.js index 45e00b7ff4..7797dcbf24 100644 --- a/src/devtools/views/Components/ComponentFiltersModal.js +++ b/src/devtools/views/Components/ComponentFiltersModal.js @@ -41,6 +41,7 @@ import type { export default function ComponentFiltersModalWrapper(_: {||}) { const store = useContext(StoreContext); + const { profilerStore } = store; const { isModalShowing, setIsModalShowing } = useContext( ComponentFiltersModalContext @@ -50,13 +51,13 @@ export default function ComponentFiltersModalWrapper(_: {||}) { // If necessary, we could support this- but it doesn't seem like a necessary use case. const isProfilingSubscription = useMemo( () => ({ - getCurrentValue: () => store.isProfiling, + getCurrentValue: () => profilerStore.isProfiling, subscribe: (callback: Function) => { - store.addListener('isProfiling', callback); - return () => store.removeListener('isProfiling', callback); + profilerStore.addListener('isProfiling', callback); + return () => profilerStore.removeListener('isProfiling', callback); }, }), - [store] + [profilerStore] ); const isProfiling = useSubscription(isProfilingSubscription); if (isProfiling && isModalShowing) { diff --git a/src/devtools/views/Components/ToggleComponentFiltersModalButton.js b/src/devtools/views/Components/ToggleComponentFiltersModalButton.js index 189454d349..e1d237f33a 100644 --- a/src/devtools/views/Components/ToggleComponentFiltersModalButton.js +++ b/src/devtools/views/Components/ToggleComponentFiltersModalButton.js @@ -14,6 +14,7 @@ import type { ComponentFilter } from 'src/types'; export default function ToggleComponentFiltersModalButton() { const store = useContext(StoreContext); + const { profilerStore } = store; const { isModalShowing, setIsModalShowing } = useContext( ComponentFiltersModalContext @@ -23,13 +24,13 @@ export default function ToggleComponentFiltersModalButton() { // If necessary, we could support this- but it doesn't seem like a necessary use case. const isProfilingSubscription = useMemo( () => ({ - getCurrentValue: () => store.isProfiling, + getCurrentValue: () => profilerStore.isProfiling, subscribe: (callback: Function) => { - store.addListener('isProfiling', callback); - return () => store.removeListener('isProfiling', callback); + profilerStore.addListener('isProfiling', callback); + return () => profilerStore.removeListener('isProfiling', callback); }, }), - [store] + [profilerStore] ); const isProfiling = useSubscription(isProfilingSubscription); diff --git a/src/devtools/views/Profiler/ClearProfilingDataButton.js b/src/devtools/views/Profiler/ClearProfilingDataButton.js index a8be6b0f6c..c99f370fce 100644 --- a/src/devtools/views/Profiler/ClearProfilingDataButton.js +++ b/src/devtools/views/Profiler/ClearProfilingDataButton.js @@ -8,13 +8,14 @@ import { StoreContext } from '../context'; export default function ClearProfilingDataButton() { const store = useContext(StoreContext); - const { isProfiling } = useContext(ProfilerContext); + const { hasProfilingData, isProfiling } = useContext(ProfilerContext); + const { profilerStore } = store; - const clear = useCallback(() => store.clearProfilingData(), [store]); + const clear = useCallback(() => profilerStore.clear(), [profilerStore]); return (