Adding polling and initial stab at not serializing duplicate inspected Element props
Brian Vaughn committed
Apr 20, 2019 at 10:35 UTC
957c389566794307e663f53964ee181c7ce64df5
4 files changed
+125
-60
src/backend/renderer.js
+27
@@ -959,6 +959,20 @@ export function attach(
959
debug('updateFiberRecursively()', nextFiber, parentFiber);
960
}
961
const shouldIncludeInTree = !shouldFilterFiber(nextFiber);
962
+
963
+ // If this is the most recently inspected Fiber, take note of whether it was part of the new commit.
964
+ // If not, we can avoid re-serializing its props and state if asked again.
965
+ // Note that we avoid even comparing IDs for fibers not in the tree,
966
+ // so that we don't inadvertantly add them to the ID Map.
967
+ if (
968
+ shouldIncludeInTree &&
969
+ inspectedElementID !== null &&
970
+ inspectedElementID === getFiberID(getPrimaryFiber(nextFiber)) &&
971
+ nextFiber.actualDuration > 0
972
+ ) {
973
+ hasInspectedElementChanged = true;
974
+ }
975
+
976
const isSuspense = nextFiber.tag === SuspenseComponent;
977
let shouldResetChildren = false;
978
// The behavior of timed-out Suspense trees is unique.
@@ -1632,16 +1646,29 @@ export function attach(
1646
};
1647
}
1648
1649
+ let inspectedElementID: number | null = null;
1650
+ let hasInspectedElementChanged: boolean = false;
1651
+
1652
function inspectElement(id: number): InspectedElement | null {
1653
+ if (inspectedElementID === id && !hasInspectedElementChanged) {
1654
+ // Optimization: Don't resend (and reserialize) unchanged props.
1655
+ return null;
1656
+ }
1657
+
1658
+ inspectedElementID = id;
1659
+ hasInspectedElementChanged = false;
1660
+
1661
let result = inspectElementRaw(id);
1662
if (result === null) {
1663
return null;
1664
}
1665
+
1666
// TODO Review sanitization approach for the below inspectable values.
1667
result.context = cleanForBridge(result.context);
1668
result.hooks = cleanForBridge(result.hooks);
1669
result.props = cleanForBridge(result.props);
1670
result.state = cleanForBridge(result.state);
1671
+
1672
return result;
1673
}
1674
src/devtools/cache.js
+7
-1
@@ -157,7 +157,13 @@ export function createResource<Input, Key: string | number, Value>(
157
158
write(key: Key, value: Value): void {
159
const entriesForResource = ((entries.get(resource): any): Map<any, any>);
160
- entriesForResource.set(key, value);
160
+
161
+ const resolvedResult: ResolvedResult<Value> = {
162
+ status: Resolved,
163
+ value,
164
+ };
165
+
166
+ entriesForResource.set(key, resolvedResult);
167
},
168
};
169
src/devtools/views/Components/InspectedElementContext.js
+74
-44
@@ -2,7 +2,6 @@
2
3
import React, {
4
createContext,
5
- useCallback,
5
useContext,
6
useEffect,
7
useMemo,
@@ -11,6 +10,8 @@ import React, {
10
import { createResource } from '../../cache';
11
import { BridgeContext, StoreContext } from '../context';
12
import { hydrate } from 'src/hydration';
13
+import { unstable_next as next } from 'scheduler';
14
+import { TreeContext } from './TreeContext';
15
16
import type {
17
DehydratedData,
@@ -18,20 +19,10 @@ import type {
19
} from 'src/devtools/views/Components/types';
20
import type { Resource } from '../../cache';
21
21
-// TODO Something needs to poll for (unprompted) updates.
22
-
23
-// TODO The curretn approach caches resources permanently.
24
-// We won't even ask for an update if an element is reselected.
25
-// I think we need to separate the polling for an update from the suspense cache.
26
-// This way we can always resened (and poll on an interval) for the selected id,
27
-// and the cache here can just invalidate itself as responses stream in.
28
-
29
-type Params = {|
30
- id: number,
31
- rendererID: number,
32
-|};
22
+// TODO This isn't using the "two setState" pattern and updates sometimes feel janky.
23
24
type Context = {|
25
+ inspectedElementID: number | null,
26
read(id: number): InspectedElement | null,
27
|};
28
@@ -52,17 +43,57 @@ function InspectedElementContextController({ children }: Props) {
43
const bridge = useContext(BridgeContext);
44
const store = useContext(StoreContext);
45
55
- const [count, setCount] = useState(0);
46
+ const { selectedElementID } = useContext(TreeContext);
47
+ const [inspectedElement, setInspectedElement] = useState<{
48
+ id: number | null,
49
+ inspectedElement: InspectedElement | null,
50
+ }>({
51
+ id: selectedElementID,
52
+ inspectedElement: null,
53
+ });
54
+ if (inspectedElement.id !== selectedElementID) {
55
+ if (selectedElementID === null) {
56
+ setInspectedElement({
57
+ id: selectedElementID,
58
+ inspectedElement: null,
59
+ });
60
+ } else {
61
+ next(() =>
62
+ setInspectedElement({
63
+ id: selectedElementID,
64
+ inspectedElement: null,
65
+ })
66
+ );
67
+ }
68
+ }
69
+
70
+ useEffect(() => {
71
+ if (inspectedElement.id === null) {
72
+ return () => {};
73
+ }
74
+
75
+ const rendererID = store.getRendererIDForElement(inspectedElement.id);
76
+
77
+ const requestUpdate = () => {
78
+ bridge.send('inspectElement', { id: inspectedElement.id, rendererID });
79
+ };
80
+
81
+ requestUpdate();
82
+
83
+ const intervalID = setInterval(requestUpdate, 1000);
84
+
85
+ return () => clearInterval(intervalID);
86
+ }, [bridge, inspectedElement.id, store]);
87
88
const inProgressRequests = useMemo<Map<number, InProgressRequest>>(
89
() => new Map(),
90
[]
91
);
92
62
- const resource = useMemo<Resource<Params, number, InspectedElement>>(
93
+ const resource = useMemo<Resource<number, number, InspectedElement>>(
94
() =>
95
createResource(
65
- ({ id, rendererID }: Params) => {
96
+ (id: number) => {
97
let request = inProgressRequests.get(id);
98
if (request != null) {
99
return request.promise;
@@ -71,28 +102,31 @@ function InspectedElementContextController({ children }: Props) {
102
let resolveFn = ((null: any): ResolveFn);
103
const promise = new Promise(resolve => {
104
resolveFn = resolve;
74
-
75
- bridge.send('inspectElement', { id, rendererID });
105
});
106
107
inProgressRequests.set(id, { promise, resolveFn });
108
109
return promise;
110
},
82
- ({ id, rendererID }: Params) => id
111
+ (id: number) => id
112
),
84
- [bridge, inProgressRequests]
113
+ [inProgressRequests]
114
);
115
116
useEffect(() => {
88
- const onInspectedElement = (inspectedElement: InspectedElement | null) => {
89
- if (inspectedElement != null) {
90
- const id = inspectedElement.id;
91
-
92
- inspectedElement.context = hydrateHelper(inspectedElement.context);
93
- inspectedElement.hooks = hydrateHelper(inspectedElement.hooks);
94
- inspectedElement.props = hydrateHelper(inspectedElement.props);
95
- inspectedElement.state = hydrateHelper(inspectedElement.state);
117
+ const onInspectedElement = (
118
+ inspectedElementRaw: InspectedElement | null
119
+ ) => {
120
+ if (inspectedElementRaw != null) {
121
+ const id = inspectedElementRaw.id;
122
+
123
+ const inspectedElement = (({
124
+ ...inspectedElementRaw,
125
+ context: hydrateHelper(inspectedElementRaw.context),
126
+ hooks: hydrateHelper(inspectedElementRaw.hooks),
127
+ props: hydrateHelper(inspectedElementRaw.props),
128
+ state: hydrateHelper(inspectedElementRaw.state),
129
+ }: any): InspectedElement);
130
131
const request = inProgressRequests.get(id);
132
if (request != null) {
@@ -101,8 +135,10 @@ function InspectedElementContextController({ children }: Props) {
135
} else {
136
resource.write(id, inspectedElement);
137
104
- // Schedule update with React.
105
- setCount(count => count + 1);
138
+ // Schedule update with React if necessary.
139
+ setInspectedElement(prevState =>
140
+ prevState.id === id ? { id, inspectedElement } : prevState
141
+ );
142
}
143
}
144
};
@@ -111,22 +147,16 @@ function InspectedElementContextController({ children }: Props) {
147
return () => bridge.removeListener('inspectElement', onInspectedElement);
148
}, [bridge, inProgressRequests, resource]);
149
114
- const read = useCallback(
115
- (id: number) => {
116
- const rendererID = store.getRendererIDForElement(id);
117
- if (rendererID != null) {
118
- return resource.read({ id, rendererID });
119
- } else {
120
- return null;
121
- }
122
- },
123
- [resource, store]
150
+ // We intentionally use the broader inspectedElement object, rather than the id,
151
+ // to enable updates to be scheduled with React after the cache has been invalidated.
152
+ const value = useMemo(
153
+ () => ({
154
+ inspectedElementID: inspectedElement.id,
155
+ read: resource.read,
156
+ }),
157
+ [inspectedElement, resource.read]
158
);
159
126
- // "count" is intentionally passed so that it recreates the memoized object.
127
- // eslint-disable-next-line react-hooks/exhaustive-deps
128
- const value = useMemo(() => ({ read }), [count, read]);
129
-
160
return (
161
<InspectedElementContext.Provider value={value}>
162
{children}
src/devtools/views/Components/SelectedElement.js
+17
-15
@@ -22,51 +22,53 @@ import type { Element, InspectedElement } from './types';
22
export type Props = {||};
23
24
export default function SelectedElement(_: Props) {
25
- const { selectedElementID, viewElementSource } = useContext(TreeContext);
25
+ const { viewElementSource } = useContext(TreeContext);
26
const bridge = useContext(BridgeContext);
27
const store = useContext(StoreContext);
28
29
- const { read } = useContext(InspectedElementContext);
29
+ const { inspectedElementID, read } = useContext(InspectedElementContext);
30
31
const element =
32
- selectedElementID !== null ? store.getElementByID(selectedElementID) : null;
32
+ inspectedElementID !== null
33
+ ? store.getElementByID(inspectedElementID)
34
+ : null;
35
36
const inspectedElement =
35
- selectedElementID != null ? read(selectedElementID) : null;
37
+ inspectedElementID != null ? read(inspectedElementID) : null;
38
39
const highlightElement = useCallback(() => {
38
- if (element !== null && selectedElementID !== null) {
39
- const rendererID = store.getRendererIDForElement(selectedElementID);
40
+ if (element !== null && inspectedElementID !== null) {
41
+ const rendererID = store.getRendererIDForElement(inspectedElementID);
42
if (rendererID !== null) {
43
bridge.send('highlightElementInDOM', {
44
displayName: element.displayName,
45
hideAfterTimeout: true,
44
- id: selectedElementID,
46
+ id: inspectedElementID,
47
openNativeElementsPanel: true,
48
rendererID,
49
scrollIntoView: true,
50
});
51
}
52
}
51
- }, [bridge, element, selectedElementID, store]);
53
+ }, [bridge, element, inspectedElementID, store]);
54
55
const logElement = useCallback(() => {
54
- if (selectedElementID !== null) {
55
- const rendererID = store.getRendererIDForElement(selectedElementID);
56
+ if (inspectedElementID !== null) {
57
+ const rendererID = store.getRendererIDForElement(inspectedElementID);
58
if (rendererID !== null) {
59
bridge.send('logElementToConsole', {
58
- id: selectedElementID,
60
+ id: inspectedElementID,
61
rendererID,
62
});
63
}
64
}
63
- }, [bridge, selectedElementID, store]);
65
+ }, [bridge, inspectedElementID, store]);
66
67
const viewSource = useCallback(() => {
66
- if (viewElementSource != null && selectedElementID !== null) {
67
- viewElementSource(selectedElementID);
68
+ if (viewElementSource != null && inspectedElementID !== null) {
69
+ viewElementSource(inspectedElementID);
70
}
69
- }, [selectedElementID, viewElementSource]);
71
+ }, [inspectedElementID, viewElementSource]);
72
73
if (element === null) {
74
return (