Moved logic to only send updated filters across Bridge to the Store (and added tests)
Brian Vaughn committed
Jun 10, 2019 at 15:36 UTC
dd748ef574b1d7c3993d87e91e7a9122bcb042bf
4 files changed
+70
-25
src/__tests__/setupTests.js
+6
-2
@@ -11,7 +11,10 @@ env.beforeEach(() => {
11
const Bridge = require('src/bridge').default;
12
const Store = require('src/devtools/store').default;
13
const { installHook } = require('src/hook');
14
- const { getDefaultComponentFilters } = require('src/utils');
14
+ const {
15
+ getDefaultComponentFilters,
16
+ saveComponentFilters,
17
+ } = require('src/utils');
18
19
// Fake timers let us flush Bridge operations between setup and assertions.
20
jest.useFakeTimers();
@@ -26,7 +29,8 @@ env.beforeEach(() => {
29
originalConsoleError.apply(console, args);
30
};
31
29
- // Avoid "Invalid component filters" warning.
32
+ // Initialize filters to a known good state.
33
+ saveComponentFilters(getDefaultComponentFilters());
34
global.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = getDefaultComponentFilters();
35
36
installHook(global);
src/__tests__/storeComponentFilters-test.js
+33
@@ -1,5 +1,6 @@
1
// @flow
2
3
+import type Bridge from 'src/bridge';
4
import type Store from 'src/devtools/store';
5
6
describe('Store component filters', () => {
@@ -7,6 +8,7 @@ describe('Store component filters', () => {
8
let ReactDOM;
9
let TestUtils;
10
let Types;
11
+ let bridge: Bridge;
12
let store: Store;
13
let utils;
14
@@ -18,6 +20,7 @@ describe('Store component filters', () => {
20
};
21
22
beforeEach(() => {
23
+ bridge = global.bridge;
24
store = global.store;
25
store.collapseNodesByDefault = false;
26
store.componentFilters = [];
@@ -187,4 +190,34 @@ describe('Store component filters', () => {
190
191
expect(store).toMatchSnapshot('3: disable HOC filter');
192
});
193
+
194
+ it('should not send a bridge update if the set of enabled filters has not changed', () => {
195
+ act(() => (store.componentFilters = [utils.createHOCFilter(true)]));
196
+
197
+ bridge.addListener('updateComponentFilters', componentFilters => {
198
+ throw Error('Unexpected component update');
199
+ });
200
+
201
+ act(
202
+ () =>
203
+ (store.componentFilters = [
204
+ utils.createHOCFilter(false),
205
+ utils.createHOCFilter(true),
206
+ ])
207
+ );
208
+ act(
209
+ () =>
210
+ (store.componentFilters = [
211
+ utils.createHOCFilter(true),
212
+ utils.createLocationFilter('abc', false),
213
+ ])
214
+ );
215
+ act(
216
+ () =>
217
+ (store.componentFilters = [
218
+ utils.createHOCFilter(true),
219
+ utils.createElementTypeFilter(Types.ElementTypeHostComponent, false),
220
+ ])
221
+ );
222
+ });
223
});
src/devtools/store.js
+27
-2
@@ -14,6 +14,7 @@ import {
14
getSavedComponentFilters,
15
saveComponentFilters,
16
separateDisplayNameAndHOCs,
17
+ shallowDiffers,
18
utfDecodeString,
19
} from '../utils';
20
import { localStorageGetItem, localStorageSetItem } from '../storage';
@@ -242,14 +243,38 @@ export default class Store extends EventEmitter<{|
243
throw Error('Cannot modify filter preferences while profiling');
244
}
245
246
+ // Filter updates are expensive to apply (since they impact the entire tree).
247
+ // Let's determine if they've changed and avoid doing this work if they haven't.
248
+ const prevEnabledComponentFilters = this._componentFilters.filter(
249
+ filter => filter.isEnabled
250
+ );
251
+ const nextEnabledComponentFilters = value.filter(
252
+ filter => filter.isEnabled
253
+ );
254
+ let haveEnabledFiltersChanged =
255
+ prevEnabledComponentFilters.length !== nextEnabledComponentFilters.length;
256
+ if (!haveEnabledFiltersChanged) {
257
+ for (let i = 0; i < nextEnabledComponentFilters.length; i++) {
258
+ const prevFilter = prevEnabledComponentFilters[i];
259
+ const nextFilter = nextEnabledComponentFilters[i];
260
+ if (shallowDiffers(prevFilter, nextFilter)) {
261
+ haveEnabledFiltersChanged = true;
262
+ break;
263
+ }
264
+ }
265
+ }
266
+
267
this._componentFilters = value;
268
269
// Update persisted filter preferences stored in localStorage.
270
saveComponentFilters(value);
271
272
// Notify the renderer that filter prefernces have changed.
251
- // This is an expensive opreation; it unmounts and remounts the entire tree.
252
- this._bridge.send('updateComponentFilters', value);
273
+ // This is an expensive opreation; it unmounts and remounts the entire tree,
274
+ // so only do it if the set of enabled component filters has changed.
275
+ if (haveEnabledFiltersChanged) {
276
+ this._bridge.send('updateComponentFilters', value);
277
+ }
278
279
this.emit('componentFilters');
280
}
src/devtools/views/Settings/ComponentsSettings.js
+4
-21
@@ -14,7 +14,6 @@ import Store from 'src/devtools/store';
14
import Button from '../Button';
15
import ButtonIcon from '../ButtonIcon';
16
import Toggle from '../Toggle';
17
-import { shallowDiffers } from 'src/utils';
17
import {
18
ComponentFilterDisplayName,
19
ComponentFilterElementType,
@@ -221,7 +220,9 @@ export default function ComponentsSettings(_: {||}) {
220
);
221
222
// Filter updates are expensive to apply (since they impact the entire tree).
224
- // Only apply them on unmount, and only if they've actually changed.
223
+ // Only apply them on unmount.
224
+ // The Store will avoid doing any expensive work unless they've changed.
225
+ // We just want to batch the work in the event that they do change.
226
const componentFiltersRef = useRef<Array<ComponentFilter>>(componentFilters);
227
useEffect(() => {
228
componentFiltersRef.current = componentFilters;
@@ -229,25 +230,7 @@ export default function ComponentsSettings(_: {||}) {
230
}, [componentFilters]);
231
useEffect(
232
() => () => {
232
- const prevComponentFilters = store.componentFilters;
233
- const nextComponentFilters = componentFiltersRef.current;
234
-
235
- let haveFiltersChanged =
236
- prevComponentFilters.length !== nextComponentFilters.length;
237
- if (!haveFiltersChanged) {
238
- for (let i = 0; i < nextComponentFilters.length; i++) {
239
- if (
240
- shallowDiffers(prevComponentFilters[i], nextComponentFilters[i])
241
- ) {
242
- haveFiltersChanged = true;
243
- break;
244
- }
245
- }
246
- }
247
-
248
- if (haveFiltersChanged) {
249
- store.componentFilters = [...nextComponentFilters];
250
- }
233
+ store.componentFilters = [...componentFiltersRef.current];
234
},
235
[store]
236
);