Skip to content

Commit

Permalink
DevTools: Named hooks supports 'cheap-module-source-map'
Browse files Browse the repository at this point in the history
This is the default used in created-react-dev mode. These source maps don't support column numbers, so DevTools needs to be more lenient when matching AST nodes in this mode.
  • Loading branch information
Brian Vaughn committed Jul 13, 2021
1 parent 9fec3f2 commit a9d4bd8
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 27 deletions.
74 changes: 48 additions & 26 deletions packages/react-devtools-extensions/src/astUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,30 +32,32 @@ const AST_NODE_TYPES = Object.freeze({
function checkNodeLocation(
path: NodePath,
line: number,
column: number,
column?: number | null = null,
): boolean {
const {start, end} = path.node.loc;

if (line < start.line || line > end.line) {
return false;
}

// Column numbers are representated differently between tools/engines.
// Error.prototype.stack columns are 1-based (like most IDEs) but ASTs are 0-based.
//
// In practice this will probably never matter,
// because this code matches the 1-based Error stack location for the hook Identifier (e.g. useState)
// with the larger 0-based VariableDeclarator (e.g. [foo, setFoo] = useState())
// so the ranges should always overlap.
//
// For more info see https://github.com/facebook/react/pull/21833#discussion_r666831276
column -= 1;

if (
(line === start.line && column < start.column) ||
(line === end.line && column > end.column)
) {
return false;
if (column !== null) {
// Column numbers are representated differently between tools/engines.
// Error.prototype.stack columns are 1-based (like most IDEs) but ASTs are 0-based.
//
// In practice this will probably never matter,
// because this code matches the 1-based Error stack location for the hook Identifier (e.g. useState)
// with the larger 0-based VariableDeclarator (e.g. [foo, setFoo] = useState())
// so the ranges should always overlap.
//
// For more info see https://github.com/facebook/react/pull/21833#discussion_r666831276
column -= 1;

if (
(line === start.line && column < start.column) ||
(line === end.line && column > end.column)
) {
return false;
}
}

return true;
Expand Down Expand Up @@ -122,15 +124,35 @@ export function getHookName(
): string | null {
const hooksFromAST = getPotentialHookDeclarationsFromAST(originalSourceAST);

const potentialReactHookASTNode = hooksFromAST.find(node => {
const nodeLocationCheck = checkNodeLocation(
node,
originalSourceLineNumber,
originalSourceColumnNumber,
);
const hookDeclaractionCheck = isConfirmedHookDeclaration(node);
return nodeLocationCheck && hookDeclaractionCheck;
});
let potentialReactHookASTNode = null;
if (originalSourceColumnNumber === 0) {
// This most likely indicates a source map type like 'cheap-module-source-map'
// that intentionally drops column numbers for compilation speed in DEV builds.
// In this case, we can assume there's probably only one hook per line (true in most cases)
// and just fail if we find more than one match.
const matchingNodes = hooksFromAST.filter(node => {
const nodeLocationCheck = checkNodeLocation(
node,
originalSourceLineNumber,
);
const hookDeclaractionCheck = isConfirmedHookDeclaration(node);
return nodeLocationCheck && hookDeclaractionCheck;
});

if (matchingNodes.length === 1) {
potentialReactHookASTNode = matchingNodes[0];
}
} else {
potentialReactHookASTNode = hooksFromAST.find(node => {
const nodeLocationCheck = checkNodeLocation(
node,
originalSourceLineNumber,
originalSourceColumnNumber,
);
const hookDeclaractionCheck = isConfirmedHookDeclaration(node);
return nodeLocationCheck && hookDeclaractionCheck;
});
}

if (!potentialReactHookASTNode) {
return null;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-extensions/src/parseHookNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ function fetchFile(url: string): Promise<string> {
},
error => {
if (__DEBUG__) {
console.log(`fetchFile() Could not fetch file "${error.message}"`);
console.log(`fetchFile() Could not fetch file: ${error.message}`);
}
reject(null);
},
Expand Down

0 comments on commit a9d4bd8

Please sign in to comment.