Clear profiling data for root when the root is unmounted
Fixes https://github.com/bvaughn/react-devtools-experimental/issues/232
Ivan Babak committed
May 1, 2019 at 01:44 UTC
cd89c0d75f6e323a3e3fc16ef9e2d09dc93675ed
3 files changed
+133
-21
src/__tests__/__snapshots__/profiler-test.js.snap
new
+31
@@ -0,0 +1,31 @@
1
+// Jest Snapshot v1, https://goo.gl/fbAQLP
2
+
3
+exports[`Profiler should start and stop profiling, handle root unmounting: 1: mount 1`] = `
4
+[root]
5
+ ▸ <Parent key="A">
6
+[root]
7
+ ▸ <Parent key="B">
8
+`;
9
+
10
+exports[`Profiler should start and stop profiling, handle root unmounting: 2: profiling started 1`] = `
11
+[root]
12
+ ▸ <Parent key="A">
13
+[root]
14
+ ▸ <Parent key="B">
15
+`;
16
+
17
+exports[`Profiler should start and stop profiling, handle root unmounting: 3: update 1`] = `
18
+[root]
19
+ ▸ <Parent key="A">
20
+[root]
21
+ ▸ <Parent key="B">
22
+`;
23
+
24
+exports[`Profiler should start and stop profiling, handle root unmounting: 4: unmount B 1`] = `
25
+[root]
26
+ ▸ <Parent key="A">
27
+`;
28
+
29
+exports[`Profiler should start and stop profiling, handle root unmounting: 5: unmount A 1`] = ``;
30
+
31
+exports[`Profiler should start and stop profiling, handle root unmounting: 6: profiling stopped 1`] = ``;
src/__tests__/profiler-test.js
new
+60
@@ -0,0 +1,60 @@
1
+// @flow
2
+
3
+describe('Profiler', () => {
4
+ let React;
5
+ let ReactDOM;
6
+ let TestUtils;
7
+ let store;
8
+
9
+ const act = (callback: Function) => {
10
+ TestUtils.act(() => {
11
+ callback();
12
+ });
13
+ jest.runAllTimers(); // Flush Bridge operations
14
+ };
15
+
16
+ beforeEach(() => {
17
+ store = global.store;
18
+
19
+ React = require('react');
20
+ ReactDOM = require('react-dom');
21
+ TestUtils = require('react-dom/test-utils');
22
+ });
23
+
24
+ it('should start and stop profiling, handle root unmounting', async () => {
25
+ const Parent = ({ count }) =>
26
+ new Array(count).fill(true).map((_, index) => <Child key={index} />);
27
+ const Child = () => <div>Hi!</div>;
28
+
29
+ const containerA = document.createElement('div');
30
+ const containerB = document.createElement('div');
31
+
32
+ act(() => {
33
+ ReactDOM.render(<Parent key="A" count={3} />, containerA);
34
+ ReactDOM.render(<Parent key="B" count={2} />, containerB);
35
+ });
36
+ expect(store).toMatchSnapshot('1: mount');
37
+
38
+ act(() => {
39
+ store.startProfiling();
40
+ });
41
+ expect(store).toMatchSnapshot('2: profiling started');
42
+
43
+ act(() => {
44
+ ReactDOM.render(<Parent key="A" count={4} />, containerA);
45
+ ReactDOM.render(<Parent key="B" count={1} />, containerB);
46
+ });
47
+ expect(store).toMatchSnapshot('3: update');
48
+
49
+ act(() => ReactDOM.unmountComponentAtNode(containerB));
50
+ expect(store).toMatchSnapshot('4: unmount B');
51
+
52
+ act(() => ReactDOM.unmountComponentAtNode(containerA));
53
+ expect(store).toMatchSnapshot('5: unmount A');
54
+
55
+ act(() => {
56
+ store.stopProfiling();
57
+ });
58
+ expect(store).toMatchSnapshot('6: profiling stopped');
59
+ });
60
+});
src/devtools/store.js
+42
-21
@@ -185,27 +185,31 @@ export default class Store extends EventEmitter {
185
186
assertEmptyMaps() {
187
// This is only used in tests to avoid memory leaks.
188
- if (this._idToElement.size !== 0) {
189
- throw new Error('Expected _idToElement to be empty.');
190
- }
191
- if (this._ownersMap.size !== 0) {
192
- throw new Error('Expected _ownersMap to be empty.');
193
- }
194
- if (this._profilingOperationsByRootID.size !== 0) {
195
- throw new Error('Expected _profilingOperationsByRootID to be empty.');
196
- }
197
- if (this._profilingScreenshotsByRootID.size !== 0) {
198
- throw new Error('Expected _profilingScreenshotsByRootID to be empty.');
199
- }
200
- if (this._profilingSnapshotsByElementID.size !== 0) {
201
- throw new Error('Expected _profilingSnapshotsByElementID to be empty.');
202
- }
203
- if (this._rootIDToCapabilities.size !== 0) {
204
- throw new Error('Expected _rootIDToCapabilities to be empty.');
205
- }
206
- if (this._rootIDToRendererID.size !== 0) {
207
- throw new Error('Expected _rootIDToRendererID to be empty.');
208
- }
188
+ const assertOneMap = (mapName, map) => {
189
+ if (map.size !== 0) {
190
+ throw new Error(
191
+ `Expected ${mapName} to be empty, got ${
192
+ map.size
193
+ }: ${require('util').inspect(this, { depth: 20 })}`
194
+ );
195
+ }
196
+ };
197
+ assertOneMap('_idToElement', this._idToElement);
198
+ assertOneMap('_ownersMap', this._ownersMap);
199
+ assertOneMap(
200
+ '_profilingOperationsByRootID',
201
+ this._profilingOperationsByRootID
202
+ );
203
+ assertOneMap(
204
+ '_profilingScreenshotsByRootID',
205
+ this._profilingScreenshotsByRootID
206
+ );
207
+ assertOneMap(
208
+ '_profilingSnapshotsByElementID',
209
+ this._profilingSnapshotsByElementID
210
+ );
211
+ assertOneMap('_rootIDToCapabilities', this._rootIDToCapabilities);
212
+ assertOneMap('_rootIDToRendererID', this._rootIDToRendererID);
213
}
214
215
get captureScreenshots(): boolean {
@@ -665,6 +669,15 @@ export default class Store extends EventEmitter {
669
}
670
};
671
672
+ _clearProfilingSnapshotRecursive = (elementID: number) => {
673
+ const element = this.getElementByID(elementID);
674
+ if (element !== null) {
675
+ this._profilingSnapshotsByElementID.delete(elementID);
676
+
677
+ element.children.forEach(this._clearProfilingSnapshotRecursive);
678
+ }
679
+ };
680
+
681
_adjustParentTreeWeight = (
682
parentElement: Element | null,
683
weightDelta: number
@@ -876,6 +889,11 @@ export default class Store extends EventEmitter {
889
throw new Error(`Node ${id} was removed before its children.`);
890
}
891
892
+ // The following call depends on `getElementByID`
893
+ // which depends on the element being in `_idToElement`,
894
+ // so we have to do it before removing the element from `_idToElement`.
895
+ this._clearProfilingSnapshotRecursive(id);
896
+
897
this._idToElement.delete(id);
898
899
let parentElement = null;
@@ -888,6 +906,9 @@ export default class Store extends EventEmitter {
906
this._rootIDToRendererID.delete(id);
907
this._rootIDToCapabilities.delete(id);
908
909
+ this._profilingOperationsByRootID.delete(id);
910
+ this._profilingScreenshotsByRootID.delete(id);
911
+
912
haveRootsChanged = true;
913
} else {
914
if (__DEBUG__) {