Skip to content

Commit

Permalink
[DevTools] Hook names are correctly extracted when parsing nested hoo…
Browse files Browse the repository at this point in the history
…k calls (#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)
  • Loading branch information
Juan authored Aug 5, 2021
1 parent a8725a3 commit b9934d6
Show file tree
Hide file tree
Showing 15 changed files with 237 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/
const {useMemo, useState} = require('react');

function Component(props) {
const InnerComponent = useMemo(() => () => {
const [state] = useState(0);

return state;
});
props.callback(InnerComponent);

return null;
}

module.exports = {Component};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit b9934d6

Please sign in to comment.