Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit be1e3bd

Browse files
Juanzhengjitf
authored andcommittedApr 15, 2022
[DevTools] Hook names are correctly extracted when parsing nested hook calls (facebook#22037)
## Summary This commit fixes an issue where DevTools would currently not correctly extract the hook names for a hook call when the hook call was nested under *another* hook call, e.g.: ```javascript function Component() { const InnerComponent = useMemo(() => () => { const [state, setState] = useState(0); return state; }); return null; }; ``` Although it seems pretty rare to encounter this case in actual product code, DevTools wasn't handling it correctly: **Expected Names:** - `InnerComponent` for the `useMemo()` call. - `state` for the `useState()` call. **Actual** - `InnerComponent` for the `useMemo()` call. - `InnerComponent` for the `useState()` call. The reason that we were extracting the name for the nested hook call incorrectly is that the `checkNodeLocation` function (which attempts to check if the location of the hook matches the location in the original source code), was too "lenient" and would return a match even if the start lines of the locations didn't match. Specifically, for our example, it would consider that the location of the outer hook call "matched" the location of the inner hook call (even though they were on different lines), and would then return the wrong hook name. ### Fix The fix in this commit is to update the `checkNodeLocation` function to more strictly check for matching start lines. The assumption here is that if 2 locations are on different starting lines, they can't possibly correspond to the same hook call. ## Test Plan - yarn flow - Tests still pass - yarn test - yarn test-build-devtools - new regression tests added - named hooks still work on manual test of browser extension on a few different apps (code sandbox, create-react-app, internally). ![image](https://user-images.githubusercontent.com/1271509/128409571-d62e0a74-6b7b-4c3f-ad86-6799ecd71962.png) ![image](https://user-images.githubusercontent.com/1271509/128409943-f898f27b-67ab-4260-a931-40d9c1942395.png) ![image](https://user-images.githubusercontent.com/1271509/128410326-79a0f822-55b1-4b90-a9b9-78f13fa0b5c5.png)
1 parent be9ffae commit be1e3bd

File tree

15 files changed

+237
-11
lines changed

15 files changed

+237
-11
lines changed
 
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
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+
const {useMemo, useState} = require('react');
10+
11+
function Component(props) {
12+
const InnerComponent = useMemo(() => () => {
13+
const [state] = useState(0);
14+
15+
return state;
16+
});
17+
props.callback(InnerComponent);
18+
19+
return null;
20+
}
21+
22+
module.exports = {Component};

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/bundle/index.js

Lines changed: 35 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/bundle/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/external/ComponentWithNestedHooks.js

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/external/ComponentWithNestedHooks.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/external/index.js

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/external/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/inline/ComponentWithNestedHooks.js

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/inline/index.js

Lines changed: 9 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/no-columns/ComponentWithNestedHooks.js

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-devtools-extensions/src/__tests__/__source__/__compiled__/no-columns/index.js

Lines changed: 9 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
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+
const {useMemo, useState} = require('react');
10+
11+
function Component(props) {
12+
const InnerComponent = useMemo(() => () => {
13+
const [state] = useState(0);
14+
15+
return state;
16+
});
17+
props.callback(InnerComponent);
18+
19+
return null;
20+
};
21+
22+
module.exports = {Component};

‎packages/react-devtools-extensions/src/__tests__/__source__/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export {Component as ComponentUsingHooksIndirectly} from './ComponentUsingHooksI
1111
export {Component as ComponentWithCustomHook} from './ComponentWithCustomHook';
1212
export {Component as ComponentWithExternalCustomHooks} from './ComponentWithExternalCustomHooks';
1313
export {Component as ComponentWithMultipleHooksPerLine} from './ComponentWithMultipleHooksPerLine';
14+
export {Component as ComponentWithNestedHooks} from './ComponentWithNestedHooks';
1415
export {Component as ContainingStringSourceMappingURL} from './ContainingStringSourceMappingURL';
1516
export {Component as Example} from './Example';
1617
export {Component as InlineRequire} from './InlineRequire';

‎packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,20 @@ describe('parseHookNames', () => {
152152
expectHookNamesToEqual(hookNames, ['count', 'darkMode', 'isDarkMode']);
153153
});
154154

155+
it('should parse names for code using nested hooks', async () => {
156+
const Component = require('./__source__/__untransformed__/ComponentWithNestedHooks')
157+
.Component;
158+
let InnerComponent;
159+
const hookNames = await getHookNamesForComponent(Component, {
160+
callback: innerComponent => {
161+
InnerComponent = innerComponent;
162+
},
163+
});
164+
const innerHookNames = await getHookNamesForComponent(InnerComponent);
165+
expectHookNamesToEqual(hookNames, ['InnerComponent']);
166+
expectHookNamesToEqual(innerHookNames, ['state']);
167+
});
168+
155169
it('should return null for custom hooks without explicit names', async () => {
156170
const Component = require('./__source__/__untransformed__/ComponentWithUnnamedCustomHooks')
157171
.Component;
@@ -261,6 +275,35 @@ describe('parseHookNames', () => {
261275
); // simulated Webpack 'cheap-module-source-map'
262276
});
263277

278+
it('should work when code is using nested hooks', async () => {
279+
async function test(path, name = 'Component') {
280+
const Component = require(path)[name];
281+
let InnerComponent;
282+
const hookNames = await getHookNamesForComponent(Component, {
283+
callback: innerComponent => {
284+
InnerComponent = innerComponent;
285+
},
286+
});
287+
const innerHookNames = await getHookNamesForComponent(InnerComponent);
288+
expectHookNamesToEqual(hookNames, [
289+
'InnerComponent', // useMemo()
290+
]);
291+
expectHookNamesToEqual(innerHookNames, [
292+
'state', // useState()
293+
]);
294+
}
295+
296+
await test('./__source__/__compiled__/inline/ComponentWithNestedHooks'); // inline source map
297+
await test('./__source__/__compiled__/external/ComponentWithNestedHooks'); // external source map
298+
await test(
299+
'./__source__/__compiled__/bundle',
300+
'ComponentWithNestedHooks',
301+
); // bundle source map
302+
await test(
303+
'./__source__/__compiled__/no-columns/ComponentWithNestedHooks',
304+
); // simulated Webpack 'cheap-module-source-map'
305+
});
306+
264307
it('should work for external hooks', async () => {
265308
async function test(path, name = 'Component') {
266309
const Component = require(path)[name];

‎packages/react-devtools-extensions/src/astUtils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function checkNodeLocation(
3636
): boolean {
3737
const {start, end} = path.node.loc;
3838

39-
if (line < start.line || line > end.line) {
39+
if (line !== start.line) {
4040
return false;
4141
}
4242

0 commit comments

Comments
 (0)
Please sign in to comment.