Removed throttling. It's too error prone because of mutations (Fibers). We'll optimize later by using the effects list for updates.
Brian Vaughn committed
Jan 24, 2019 at 13:45 UTC
13ca26e1536079b7eb9dd38f47f1c0173ca265cb
3 files changed
+13
-47
src/backend/agent.js
+8
-46
@@ -26,17 +26,9 @@ export default class Agent extends EventEmitter {
26
_idToFiber: Map<string, Fiber> = new Map();
27
_idToRendererData: Map<string, RendererData> = new Map();
28
_idToRendererID: Map<string, RendererID> = new Map();
29
- _isRenderInProgress: boolean = false;
29
_rendererInterfaces: { [key: RendererID]: RendererInterface } = {};
30
_roots: Set<RendererID> = new Set();
31
33
- // Tree updates are debounced to reduce bridge traffic and improve performance.
34
- // This reduces the potential negative impact that DevTools has on React performance.
35
- // Eventual consistency is good enough for the Elements tree.
36
- _lastCrawlTime: number = -THROTTLE_BY_MS;
37
- _pendingRoots: Set<string> = new Set();
38
- _pendingRootTimeoutID: TimeoutID | null = null;
39
-
32
addBridge(bridge: Bridge) {
33
// TODO Listen to bridge for things like selection.
34
// bridge.on('...'), this...);
@@ -83,25 +75,6 @@ export default class Agent extends EventEmitter {
75
}
76
}
77
86
- _crawlPendingRoots(): void {
87
- this._pendingRoots.forEach(id => this._crawlRoot(id));
88
- this._pendingRoots.clear();
89
- this._lastCrawlTime = performance.now();
90
- }
91
-
92
- _crawlRoot(id: string): void {
93
- const data = ((this._idToRendererData.get(id): any): RendererData);
94
-
95
- // TODO: Can we use the effects list on update for a faster path?
96
- this._createOrUpdateElement(id, data);
97
-
98
- if (!this._roots.has(id)) {
99
- this._roots.add(id);
100
- debug('emit("root")', id);
101
- this.emit('root', id);
102
- }
103
- }
104
-
78
_createOrUpdateElement(id: string, data: RendererData): void {
79
const prevElement: ?Element = this._idToElement.get(id);
80
const nextElement: Element = {
@@ -129,13 +102,6 @@ export default class Agent extends EventEmitter {
102
}
103
}
104
132
- _maybeCrawlRoot = () => {
133
- this._pendingRootTimeoutID = null;
134
- if (!this._isRenderInProgress) {
135
- this._crawlPendingRoots();
136
- }
137
- };
138
-
105
onHookMount = ({
106
data,
107
fiber,
@@ -162,17 +128,14 @@ export default class Agent extends EventEmitter {
128
}) => {
129
const id = this._getId(fiber);
130
165
- this._isRenderInProgress = false;
166
- this._pendingRoots.add(id);
167
-
168
- const delta = performance.now() - this._lastCrawlTime;
169
- if (delta >= THROTTLE_BY_MS) {
170
- this._crawlPendingRoots();
171
- } else if (this._pendingRootTimeoutID === null) {
172
- this._pendingRootTimeoutID = setTimeout(
173
- this._maybeCrawlRoot,
174
- THROTTLE_BY_MS - delta
175
- );
131
+ // TODO: Can we use the effects list on update for a faster path?
132
+ // Mounts (Placements) and unmounts (Deletions) are on the child; need to update parent children in that case.
133
+ this._createOrUpdateElement(id, data);
134
+
135
+ if (!this._roots.has(id)) {
136
+ this._roots.add(id);
137
+ debug('emit("root")', id);
138
+ this.emit('root', id);
139
}
140
};
141
@@ -198,7 +161,6 @@ export default class Agent extends EventEmitter {
161
onHookUpdate = ({ data, fiber }: { data: RendererData, fiber: Fiber }) => {
162
const id = this._getId(fiber);
163
201
- this._isRenderInProgress = true;
164
this._idToRendererData.set(id, data);
165
};
166
}
src/devtools/views/Element.js
+1
-1
@@ -17,7 +17,7 @@ export default function Element({ depth, id }: Props) {
17
18
const { children, displayName, key } = element;
19
20
- // TODO: Toggle open/close state
20
+ // TODO: Add state for toggling element open/close
21
22
return (
23
<Fragment>
src/devtools/views/hooks.js
+4
@@ -10,6 +10,8 @@ export function useElement(store: Store, id: string): Element {
10
((store.getElement(id): any): Element)
11
);
12
13
+ // TODO: We might still miss updates in concurrent mode.
14
+ // We should just useEffect and do a sync comparison (like in create-subscription) to handle this.
15
useLayoutEffect(() => {
16
const handler = () => setElement(((store.getElement(id): any): Element));
17
store.addListener(id, handler);
@@ -22,6 +24,8 @@ export function useElement(store: Store, id: string): Element {
24
export function useRoots(store: Store): Array<string> {
25
const [roots, setRoots] = useState<Array<string>>(Array.from(store.roots));
26
27
+ // TODO: We might still miss updates in concurrent mode.
28
+ // We should just useEffect and do a sync comparison (like in create-subscription) to handle this.
29
useLayoutEffect(() => {
30
const handler = () => setRoots(Array.from(store.roots));
31
store.addListener('roots', handler);