Replaced SearchContext useStates with useReducer and fixed an edge-case search bug
Brian Vaughn committed
Feb 6, 2019 at 16:31 UTC
e77ba003362f24cf4d9af3b36411829c9413be7e
8 files changed
+166
-127
src/backend/agent.js
-5
@@ -59,9 +59,4 @@ export default class Agent extends EventEmitter {
59
debug('onHookOperations', operations);
60
this._bridge.send('operations', operations, [operations.buffer]);
61
};
62
-
63
- onHookRootCommitted = (rootID: string) => {
64
- debug('onHookRootCommitted', rootID);
65
- this._bridge.send('rootCommitted', rootID);
66
- };
62
}
src/backend/index.js
-1
@@ -24,7 +24,6 @@ export function initBackend(hook: Hook, agent: Agent, global: Object): void {
24
),
25
26
hook.sub('operations', agent.onHookOperations),
27
- hook.sub('rootCommitted', agent.onHookRootCommitted),
27
28
// TODO Add additional subscriptions required for profiling mode
29
];
src/backend/renderer.js
+2
-5
@@ -524,13 +524,10 @@ export function attach(
524
addOperation(idArray, true);
525
526
// Let the frontend know about tree operations.
527
+ // The first value in this array will identify which root it corresponds to,
528
+ // so we do no longer need to dispatch a separate root-committed event.
529
hook.emit('operations', pendingOperations);
530
pendingOperations = new Uint32Array(0);
529
-
530
- // Let the frontend know that we're done working on this root.
531
- // Technically this could be inferred, but it's better to explicitly do this for the case of multi roots.
532
- // Else the frontend would need to traverse the tree to identify which updates corresponded to which roots.
533
- hook.emit('rootCommitted', getFiberID(getPrimaryFiber(root.current)));
531
}
532
533
function enqueueMount(fiber: Fiber, parentFiber: Fiber | null) {
src/devtools/store.js
+10
-10
@@ -49,7 +49,6 @@ export default class Store extends EventEmitter {
49
debug('constructor', 'subscribing to Bridge');
50
51
bridge.on('operations', this.onBridgeOperations);
52
- bridge.on('rootCommitted', this.onBridgeRootCommitted);
52
}
53
54
get numElements(): number {
@@ -196,6 +195,9 @@ export default class Store extends EventEmitter {
195
196
const rendererID = operations[0];
197
198
+ let addedElementIDs: Uint32Array = new Uint32Array(0);
199
+ let removedElementIDs: Uint32Array = new Uint32Array(0);
200
+
201
let i = 1;
202
while (i < operations.length) {
203
let id: number = ((null: any): number);
@@ -278,7 +280,9 @@ export default class Store extends EventEmitter {
280
281
this._idToElement.set(id, element);
282
281
- this.emit('elementAdded', element);
283
+ // Track newly addewd items so search results can be updated
284
+ addedElementIDs = new Uint32Array(addedElementIDs.length + 1);
285
+ addedElementIDs[addedElementIDs.length - 1] = id;
286
287
weightDelta = 1;
288
}
@@ -308,7 +312,9 @@ export default class Store extends EventEmitter {
312
);
313
}
314
311
- this.emit('elementRemoved', element);
315
+ // Track removed items so search results can be updated
316
+ removedElementIDs = new Uint32Array(removedElementIDs.length + 1);
317
+ removedElementIDs[removedElementIDs.length - 1] = id;
318
break;
319
case TREE_OPERATION_RESET_CHILDREN:
320
id = ((operations[i + 1]: any): number);
@@ -353,14 +359,8 @@ export default class Store extends EventEmitter {
359
if (haveRootsChanged) {
360
this.emit('roots');
361
}
356
- };
357
-
358
- onBridgeRootCommitted = (rootID: number) => {
359
- debug('onBridgeRootCommitted', rootID);
360
-
361
- this.emit('rootCommitted');
362
363
- // this.__printTree(rootID);
363
+ this.emit('mutated', [addedElementIDs, removedElementIDs]);
364
};
365
366
// DEBUG
src/devtools/views/Elements.js
+2
-2
@@ -41,9 +41,9 @@ export default function Elements({ bridge, browserName, themeName }: Props) {
41
handler();
42
}
43
44
- store.addListener('rootCommitted', handler);
44
+ store.addListener('mutated', handler);
45
46
- return () => store.removeListener('rootCommitted', handler);
46
+ return () => store.removeListener('mutated', handler);
47
}, [store]);
48
49
// TODO Flex wrappers below should be user resizable.
src/devtools/views/SearchContext.js
+131
-85
@@ -5,7 +5,7 @@ import React, {
5
useContext,
6
useLayoutEffect,
7
useMemo,
8
- useState,
8
+ useReducer,
9
} from 'react';
10
import { StoreContext } from './context';
11
import { SelectedElementContext } from './SelectedElementContext';
@@ -27,6 +27,29 @@ const SearchContext = createContext<SearchContextValue>(
27
// $FlowFixMe displayName is a valid attribute of React$Context
28
SearchContext.displayName = 'SearchContext';
29
30
+type State = {|
31
+ currentIndex: number | null,
32
+ ids: Array<number>,
33
+ text: string,
34
+|};
35
+
36
+type Action = {|
37
+ type: 'UPDATE',
38
+ payload: $Shape<State>,
39
+|};
40
+
41
+function reducer(state: State, action: Action) {
42
+ switch (action.type) {
43
+ case 'UPDATE':
44
+ return {
45
+ ...state,
46
+ ...action.payload,
47
+ };
48
+ default:
49
+ throw new Error();
50
+ }
51
+}
52
+
53
type Props = {|
54
children: React$Node,
55
|};
@@ -36,138 +59,161 @@ function SearchController({ children }: Props) {
59
const store = useContext(StoreContext);
60
const selectedElement = useContext(SelectedElementContext);
61
39
- const [currentIndex, setCurrentIndex] = useState<number | null>(null);
40
- const [ids, setIDs] = useState<Array<number>>([]);
41
- const [text, setText] = useState<string>('');
42
-
43
- const updateIndexHelper = (
44
- newCurrentIndex: number | null,
45
- newOrCurrentIDs: Array<number>
46
- ) => {
47
- setCurrentIndex(newCurrentIndex);
48
-
49
- // Make sure a new search result is also selected.
50
- selectedElement.id =
51
- newCurrentIndex !== null ? newOrCurrentIDs[newCurrentIndex] : null;
52
- };
62
+ const [state, dispatch] = useReducer(reducer, {
63
+ currentIndex: null,
64
+ ids: [],
65
+ text: '',
66
+ });
67
68
const value = useMemo(
69
() => ({
70
get currentIndex(): number | null {
57
- return currentIndex;
71
+ return state.currentIndex;
72
},
73
74
get ids(): Array<number> {
61
- return ids;
75
+ return state.ids;
76
},
77
78
get text(): string {
65
- return text;
79
+ return state.text;
80
},
81
82
updateCurrentIndex(newCurrentIndex: number) {
69
- updateIndexHelper(newCurrentIndex, ids);
83
+ dispatch({
84
+ type: 'UPDATE',
85
+ payload: {
86
+ currentIndex: newCurrentIndex,
87
+ },
88
+ });
89
+
90
+ // Make sure a new search result is also selected.
91
+ selectedElement.id =
92
+ newCurrentIndex !== null ? state.ids[newCurrentIndex] : null;
93
},
94
95
updateText(newText: string) {
73
- setText(newText);
96
+ const { currentIndex: oldCurrentIndex, text: oldText } = state;
97
+
98
+ let newCurrentIndex = oldCurrentIndex;
99
+ let newIDs = [];
100
101
// Find all matching elements.
76
- const newIDs = [];
102
if (newText !== '') {
103
const regExp = new RegExp(newText, 'i');
104
store.roots.forEach(rootID => {
105
searchTree(store, rootID, regExp, newIDs);
106
});
107
83
- setIDs(newIDs);
84
-
108
// If this is a refinement of a previous search, preserve the current index (unless it's no longer valid).
109
// If it's a new search, reset the index.
110
if (newIDs.length === 0) {
88
- updateIndexHelper(null, newIDs);
89
- } else if (currentIndex !== null && newText.startsWith(text)) {
90
- updateIndexHelper(
91
- Math.min(currentIndex, newIDs.length - 1),
92
- newIDs
93
- );
111
+ newCurrentIndex = null;
112
+ } else if (oldCurrentIndex !== null && newText.startsWith(oldText)) {
113
+ newCurrentIndex = Math.min(oldCurrentIndex, newIDs.length - 1);
114
} else {
95
- updateIndexHelper(0, newIDs);
115
+ newCurrentIndex = 0;
116
}
117
} else {
98
- setIDs([]);
99
- updateIndexHelper(null, newIDs);
118
+ newCurrentIndex = null;
119
}
120
+
121
+ dispatch({
122
+ type: 'UPDATE',
123
+ payload: {
124
+ currentIndex: newCurrentIndex,
125
+ ids: newIDs,
126
+ text: newText,
127
+ },
128
+ });
129
+
130
+ // Make sure a new search result is also selected.
131
+ selectedElement.id =
132
+ newCurrentIndex !== null ? newIDs[newCurrentIndex] : null;
133
},
134
}),
103
- [currentIndex, ids, text]
135
+ [state]
136
);
137
138
// Listen for changes to the tree and incrementally adjust the search results.
139
useLayoutEffect(() => {
108
- const handleElementAdded = (element: Element) => {
140
+ const handleStoreMutated = ([
141
+ addedElementIDs,
142
+ removedElementIDs,
143
+ ]: Array<Uint32Array>) => {
144
+ const { currentIndex, ids, text } = state;
145
+
146
if (!text) {
147
return;
148
}
149
113
- const { displayName, id } = element;
114
-
115
- // Add this item to the search results if it matches.
116
- const regExp = new RegExp(text, 'i');
117
- if (displayName !== null && regExp.test(displayName)) {
118
- const newElementIndex = ((store.getIndexOfElementID(id): any): number);
119
-
120
- let newIDs = null;
121
- for (let index = 0; index < ids.length; index++) {
122
- const id = ids[index];
123
- if (
124
- newElementIndex < ((store.getIndexOfElementID(id): any): number)
125
- ) {
126
- newIDs = ids
127
- .slice(0, index)
128
- .concat(id)
129
- .concat(ids.slice(index));
130
- break;
150
+ let newIDs = ids;
151
+ let newCurrentIndex = currentIndex;
152
+
153
+ removedElementIDs.forEach(id => {
154
+ // Prune this item from the search results.
155
+ const index = newIDs.indexOf(id);
156
+ if (index >= 0) {
157
+ newIDs = newIDs.slice(0, index).concat(newIDs.slice(index + 1));
158
+
159
+ // If the results are now empty, also deselect things.
160
+ if (newIDs.length === 0) {
161
+ newCurrentIndex = null;
162
+ } else if (((newCurrentIndex: any): number) >= newIDs.length) {
163
+ newCurrentIndex = newIDs.length - 1;
164
}
165
}
133
- if (newIDs === null) {
134
- newIDs = ids.concat(id);
135
- }
136
-
137
- setIDs(newIDs);
138
- updateIndexHelper(currentIndex === null ? 0 : currentIndex, newIDs);
139
- }
140
- };
141
- const handleElementRemoved = (element: Element) => {
142
- if (!text) {
143
- return;
144
- }
166
+ });
167
+
168
+ addedElementIDs.forEach(id => {
169
+ const { displayName } = ((store.getElementByID(id): any): Element);
170
+
171
+ // Add this item to the search results if it matches.
172
+ const regExp = new RegExp(text, 'i');
173
+ if (displayName !== null && regExp.test(displayName)) {
174
+ const newElementIndex = ((store.getIndexOfElementID(
175
+ id
176
+ ): any): number);
177
+
178
+ let foundMatch = false;
179
+ for (let index = 0; index < newIDs.length; index++) {
180
+ const id = newIDs[index];
181
+ if (
182
+ newElementIndex < ((store.getIndexOfElementID(id): any): number)
183
+ ) {
184
+ foundMatch = true;
185
+ newIDs = newIDs
186
+ .slice(0, index)
187
+ .concat(id)
188
+ .concat(newIDs.slice(index));
189
+ break;
190
+ }
191
+ }
192
+ if (!foundMatch) {
193
+ newIDs = newIDs.concat(id);
194
+ }
195
146
- // Prune this item from the search results.
147
- const index = ids.indexOf(element.id);
148
- if (index >= 0) {
149
- const newIDs = ids.slice(0, index).concat(ids.slice(index + 1));
150
- setIDs(newIDs);
151
-
152
- // If the results are now empty, also deselect things.
153
- if (newIDs.length === 0) {
154
- updateIndexHelper(null, newIDs);
155
- } else if (((currentIndex: any): number) < newIDs.length) {
156
- updateIndexHelper(currentIndex, newIDs);
157
- } else {
158
- updateIndexHelper(newIDs.length - 1, newIDs);
196
+ newCurrentIndex = newCurrentIndex === null ? 0 : newCurrentIndex;
197
}
160
- }
198
+ });
199
+
200
+ dispatch({
201
+ type: 'UPDATE',
202
+ payload: {
203
+ currentIndex: newCurrentIndex,
204
+ ids: newIDs,
205
+ },
206
+ });
207
+
208
+ // Make sure a new search result is also selected.
209
+ selectedElement.id =
210
+ newCurrentIndex !== null ? newIDs[newCurrentIndex] : null;
211
};
212
163
- store.addListener('elementAdded', handleElementAdded);
164
- store.addListener('elementRemoved', handleElementRemoved);
213
+ store.addListener('mutated', handleStoreMutated);
214
166
- return () => {
167
- store.removeListener('elementAdded', handleElementAdded);
168
- store.removeListener('elementRemoved', handleElementRemoved);
169
- };
170
- }, [currentIndex, ids, store, text]);
215
+ return () => store.removeListener('mutated', handleStoreMutated);
216
+ }, [state, store]);
217
218
return (
219
<SearchContext.Provider value={value}>{children}</SearchContext.Provider>
src/devtools/views/SearchInput.js
+16
-17
@@ -9,49 +9,46 @@ import styles from './SearchInput.css';
9
type Props = {||};
10
11
export default function SearchInput(props: Props) {
12
- const {
13
- currentIndex,
14
- ids,
15
- updateCurrentIndex,
16
- updateText,
17
- text,
18
- } = useContext(SearchContext);
12
+ const searchContext = useContext(SearchContext);
13
14
const inputRef = useRef();
15
16
const handleTextChange = useCallback(({ currentTarget }) => {
23
- updateText(currentTarget.value);
17
+ searchContext.updateText(currentTarget.value);
18
});
19
20
const selectNext = useCallback(() => {
21
+ const { currentIndex } = searchContext;
22
if (currentIndex !== null) {
28
- updateCurrentIndex(currentIndex + 1);
23
+ searchContext.updateCurrentIndex(currentIndex + 1);
24
}
30
- }, [currentIndex]);
25
+ }, [searchContext]);
26
27
const selectPrevious = useCallback(() => {
28
+ const { currentIndex } = searchContext;
29
if (currentIndex !== null) {
34
- updateCurrentIndex(currentIndex - 1);
30
+ searchContext.updateCurrentIndex(currentIndex - 1);
31
}
36
- }, [currentIndex]);
32
+ }, [searchContext]);
33
34
const resetSearch = useCallback(() => {
39
- updateText('');
40
- }, [updateText]);
35
+ searchContext.updateText('');
36
+ }, [searchContext]);
37
38
const handleInputKeyPress = useCallback(
39
({ key }) => {
40
if (key === 'Enter') {
41
+ const { currentIndex, ids } = searchContext;
42
if (currentIndex !== null) {
43
if (currentIndex + 1 < ids.length) {
47
- updateCurrentIndex(currentIndex + 1);
44
+ searchContext.updateCurrentIndex(currentIndex + 1);
45
} else {
49
- updateCurrentIndex(0);
46
+ searchContext.updateCurrentIndex(0);
47
}
48
}
49
}
50
},
54
- [currentIndex, ids]
51
+ [searchContext]
52
);
53
54
// Auto-focus search input
@@ -71,6 +68,8 @@ export default function SearchInput(props: Props) {
68
return () => window.removeEventListener('keydown', handleWindowKeyDown);
69
}, [inputRef]);
70
71
+ const { currentIndex, ids, text } = searchContext;
72
+
73
return (
74
<div className={styles.SearchInput}>
75
<input
src/devtools/views/SelectedElement.js
+5
-2
@@ -126,7 +126,8 @@ function useInspectedElement(id: number | null): InspectedElement | null {
126
const bridge = useContext(BridgeContext);
127
const store = useContext(StoreContext);
128
129
- const rendererID = id === null ? null : store.getRendererIDForElement(id);
129
+ const rendererID =
130
+ id === null ? null : store.getRendererIDForElement(id) || null;
131
132
const [inspectedElement, setInspectedElement] = useState(null);
133
@@ -138,7 +139,9 @@ function useInspectedElement(id: number | null): InspectedElement | null {
139
// Hide previous/stale insepected element to avoid temporarily showing the wrong values.
140
setInspectedElement(null);
141
141
- if (id === null) {
142
+ // A null id indicates that there's nothing currently selected in the tree.
143
+ // A null renderer ID indicates that the previously selected element has been unmounted.
144
+ if (id === null || rendererID === null) {
145
return () => {};
146
}
147