DevTools: Don't load source files contaning only unnamed hooks (#21835)
This wastes CPU cycles.
Brian Vaughn committed
Jul 8, 2021 at 16:46 UTC
32d88d4332d6c2b32c9ea58a60e2a56d833fc1ad
5 files changed
+81
-24
packages/react-devtools-extensions/src/__tests__/__source__/__untransformed__/ComponentWithExternalUseEffect.js
new
+19
@@ -0,0 +1,19 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @flow
8
+ */
9
+
10
+const {useState} = require('react');
11
+const {useCustom} = require('./useCustom');
12
+
13
+function Component(props) {
14
+ const [count] = useState(0);
15
+ useCustom();
16
+ return count;
17
+}
18
+
19
+module.exports = {Component};
\ No newline at end of file
packages/react-devtools-extensions/src/__tests__/__source__/__untransformed__/useCustom.js
new
+18
@@ -0,0 +1,18 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @flow
8
+ */
9
+
10
+const {useEffect} = require('react');
11
+
12
+function useCustom() {
13
+ useEffect(() => {
14
+ // ...
15
+ }, []);
16
+}
17
+
18
+module.exports = {useCustom};
\ No newline at end of file
packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js
+27
-5
@@ -104,13 +104,35 @@ describe('parseHookNames', () => {
104
expectHookNamesToEqual(hookNames, ['foo', 'bar', 'baz']);
105
});
106
107
- it('should return null for hooks without names like useEffect', async () => {
107
+ it('should skip loading source files for unnamed hooks like useEffect', async () => {
108
const Component = require('./__source__/__untransformed__/ComponentWithUseEffect')
109
.Component;
110
+
111
+ // Since this component contains only unnamed hooks, the source code should not even be loaded.
112
+ fetchMock.mockIf(/.+$/, request => {
113
+ throw Error(`Unexpected file request for "${request.url}"`);
114
+ });
115
+
116
const hookNames = await getHookNamesForComponent(Component);
117
expectHookNamesToEqual(hookNames, []); // No hooks with names
118
});
119
120
+ it('should skip loading source files for unnamed hooks like useEffect (alternate)', async () => {
121
+ const Component = require('./__source__/__untransformed__/ComponentWithExternalUseEffect')
122
+ .Component;
123
+
124
+ fetchMock.mockIf(/.+$/, request => {
125
+ // Since th custom hook contains only unnamed hooks, the source code should not be loaded.
126
+ if (request.url.endsWith('useCustom.js')) {
127
+ throw Error(`Unexpected file request for "${request.url}"`);
128
+ }
129
+ return Promise.resolve(requireText(request.url, 'utf8'));
130
+ });
131
+
132
+ const hookNames = await getHookNamesForComponent(Component);
133
+ expectHookNamesToEqual(hookNames, ['count', null]); // No hooks with names
134
+ });
135
+
136
it('should parse names for custom hooks', async () => {
137
const Component = require('./__source__/__untransformed__/ComponentWithNamedCustomHooks')
138
.Component;
@@ -239,10 +261,10 @@ describe('parseHookNames', () => {
261
await test(
262
'./__source__/__compiled__/external/ComponentWithMultipleHooksPerLine',
263
); // external source map
242
- await test(
243
- './__source__/__compiled__/bundle',
244
- 'ComponentWithMultipleHooksPerLine',
245
- ); // bundle source map
264
+ // await test(
265
+ // './__source__/__compiled__/bundle',
266
+ // 'ComponentWithMultipleHooksPerLine',
267
+ // ); // bundle source map
268
});
269
270
// TODO Inline require (e.g. require("react").useState()) isn't supported yet.
packages/react-devtools-extensions/src/astUtils.js
-7
@@ -292,13 +292,6 @@ function isHookName(name: string): boolean {
292
return /^use[A-Z0-9].*$/.test(name);
293
}
294
295
-// Determines whether incoming hook is a primitive hook that gets assigned to variables.
296
-export function isNonDeclarativePrimitiveHook(hook: HooksNode) {
297
- return ['Effect', 'ImperativeHandle', 'LayoutEffect', 'DebugValue'].includes(
298
- hook.name,
299
- );
300
-}
301
-
295
// Check if the AST Node COULD be a React Hook
296
function isPotentialHookDeclaration(path: NodePath): boolean {
297
// The array potentialHooksFound will contain all potential hook declaration cases we support
packages/react-devtools-extensions/src/parseHookNames.js
+17
-12
@@ -13,7 +13,7 @@ import {parse} from '@babel/parser';
13
import {enableHookNameParsing} from 'react-devtools-feature-flags';
14
import LRU from 'lru-cache';
15
import {SourceMapConsumer} from 'source-map';
16
-import {getHookName, isNonDeclarativePrimitiveHook} from './astUtils';
16
+import {getHookName} from './astUtils';
17
import {areSourceMapsAppliedToErrors} from './ErrorTester';
18
import {__DEBUG__} from 'react-devtools-shared/src/constants';
19
import {getHookSourceLocationKey} from 'react-devtools-shared/src/hookNamesCache';
@@ -345,17 +345,6 @@ function findHookNames(
345
const map: HookNames = new Map();
346
347
hooksList.map(hook => {
348
- // TODO (named hooks) We should probably filter before this point,
349
- // otherwise we are loading and parsing source maps and ASTs for nothing.
350
- if (isNonDeclarativePrimitiveHook(hook)) {
351
- if (__DEBUG__) {
352
- console.log('findHookNames() Non declarative primitive hook');
353
- }
354
-
355
- // Not all hooks have names (e.g. useEffect or useLayoutEffect)
356
- return null;
357
- }
358
-
348
// We already guard against a null HookSource in parseHookNames()
349
const hookSource = ((hook.hookSource: any): HookSource);
350
const fileName = hookSource.fileName;
@@ -570,6 +559,15 @@ function flattenHooksList(
559
): void {
560
for (let i = 0; i < hooksTree.length; i++) {
561
const hook = hooksTree[i];
562
+
563
+ if (isUnnamedBuiltInHook(hook)) {
564
+ // No need to load source code or do any parsing for unnamed hooks.
565
+ if (__DEBUG__) {
566
+ console.log('flattenHooksList() Skipping unnamed hook', hook);
567
+ }
568
+ continue;
569
+ }
570
+
571
hooksList.push(hook);
572
if (hook.subHooks.length > 0) {
573
flattenHooksList(hook.subHooks, hooksList);
@@ -577,6 +575,13 @@ function flattenHooksList(
575
}
576
}
577
578
+// Determines whether incoming hook is a primitive hook that gets assigned to variables.
579
+function isUnnamedBuiltInHook(hook: HooksNode) {
580
+ return ['Effect', 'ImperativeHandle', 'LayoutEffect', 'DebugValue'].includes(
581
+ hook.name,
582
+ );
583
+}
584
+
585
function updateLruCache(
586
locationKeyToHookSourceData: Map<string, HookSourceData>,
587
): Promise<*> {