Skip to content

[DevTools] Only call SourceMapConsumer.originalPositionFor once #22181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 7, 2021
Merged

[DevTools] Only call SourceMapConsumer.originalPositionFor once #22181

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 66 additions & 48 deletions packages/react-devtools-extensions/src/parseHookNames/parseSourceAndMetadata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ type HookParsedMetadata = {|
// Original source URL if there is a source map, or the same as runtimeSourceURL.
originalSourceURL: string | null,

// Line number in original source code.
originalSourceLineNumber: number | null,

// Column number in original source code.
originalSourceColumnNumber: number | null,

// APIs from source-map for parsing source maps (if detected).
sourceConsumer: SourceConsumer | null,
|};
Expand Down Expand Up @@ -151,47 +157,18 @@ function findHookNames(
return null; // Should not be reachable.
}

const {originalSourceURL, sourceConsumer} = hookParsedMetadata;

let originalSourceColumnNumber;
let originalSourceLineNumber;
if (areSourceMapsAppliedToErrors() || !sourceConsumer) {
// Either the current environment automatically applies source maps to errors,
// or the current code had no source map to begin with.
// Either way, we don't need to convert the Error stack frame locations.
originalSourceColumnNumber = columnNumber;
originalSourceLineNumber = lineNumber;
} else {
// TODO (named hooks) Refactor this read, github.com/facebook/react/pull/22181
const position = withSyncPerformanceMark(
'sourceConsumer.originalPositionFor()',
() =>
sourceConsumer.originalPositionFor({
line: lineNumber,

// Column numbers are represented differently between tools/engines.
// Error.prototype.stack columns are 1-based (like most IDEs) but ASTs are 0-based.
// For more info see https://github.com/facebook/react/issues/21792#issuecomment-873171991
column: columnNumber - 1,
}),
);

originalSourceColumnNumber = position.column;
originalSourceLineNumber = position.line;
}

if (__DEBUG__) {
console.log(
`findHookNames() mapped line ${lineNumber}->${originalSourceLineNumber} and column ${columnNumber}->${originalSourceColumnNumber}`,
);
}
const {
originalSourceURL,
originalSourceColumnNumber,
originalSourceLineNumber,
} = hookParsedMetadata;

if (
originalSourceLineNumber == null ||
originalSourceColumnNumber == null ||
originalSourceURL == null
) {
return null;
return null; // Should not be reachable.
}

let name;
Expand Down Expand Up @@ -241,6 +218,8 @@ function initializeHookParsedMetadata(
originalSourceAST: null,
originalSourceCode: null,
originalSourceURL: null,
originalSourceLineNumber: null,
originalSourceColumnNumber: null,
sourceConsumer: null,
};

Expand Down Expand Up @@ -286,21 +265,45 @@ function parseSourceAST(
return;
}

if (
hookParsedMetadata.originalSourceURL != null &&
hookParsedMetadata.originalSourceCode != null &&
hookParsedMetadata.originalSourceColumnNumber != null &&
hookParsedMetadata.originalSourceLineNumber != null
) {
// Use cached metadata.
return;
}

const {lineNumber, columnNumber} = hookSourceAndMetadata.hookSource;
if (lineNumber == null || columnNumber == null) {
throw Error('Hook source code location not found.');
}

const {metadataConsumer, sourceConsumer} = hookParsedMetadata;
const runtimeSourceCode = ((hookSourceAndMetadata.runtimeSourceCode: any): string);
let hasHookMap = false;
let originalSourceURL;
let originalSourceCode;
if (sourceConsumer !== null) {
let originalSourceColumnNumber;
let originalSourceLineNumber;
if (areSourceMapsAppliedToErrors() || sourceConsumer == null) {
// Either the current environment automatically applies source maps to errors,
// or the current code had no source map to begin with.
// Either way, we don't need to convert the Error stack frame locations.
originalSourceColumnNumber = columnNumber;
originalSourceLineNumber = lineNumber;
// There's no source map to parse here so we can just parse the original source itself.
originalSourceCode = runtimeSourceCode;
// TODO (named hooks) This mixes runtimeSourceURLs with source mapped URLs in the same cache key space.
// Namespace them?
originalSourceURL = hookSourceAndMetadata.runtimeSourceURL;
} else {
// Parse and extract the AST from the source map.
const {lineNumber, columnNumber} = hookSourceAndMetadata.hookSource;
if (lineNumber == null || columnNumber == null) {
throw Error('Hook source code location not found.');
}
// Now that the source map has been loaded,
// extract the original source for later.
// TODO (named hooks) Refactor this read, github.com/facebook/react/pull/22181
const {source} = withSyncPerformanceMark(
const {column, line, source} = withSyncPerformanceMark(
'sourceConsumer.originalPositionFor()',
() =>
sourceConsumer.originalPositionFor({
Expand All @@ -320,6 +323,8 @@ function parseSourceAST(
);
}

originalSourceColumnNumber = column;
originalSourceLineNumber = line;
// TODO (named hooks) maybe canonicalize this URL somehow?
// It can be relative if the source map specifies it that way,
// but we use it as a cache key across different source maps and there can be collisions.
Expand All @@ -331,7 +336,7 @@ function parseSourceAST(

if (__DEBUG__) {
console.groupCollapsed(
'parseSourceAST() Extracted source code from source map',
`parseSourceAST() Extracted source code from source map for "${originalSourceURL}"`,
);
console.log(originalSourceCode);
console.groupEnd();
Expand All @@ -343,24 +348,37 @@ function parseSourceAST(
) {
hasHookMap = true;
}
} else {
// There's no source map to parse here so we can just parse the original source itself.
originalSourceCode = runtimeSourceCode;
// TODO (named hooks) This mixes runtimeSourceURLs with source mapped URLs in the same cache key space.
// Namespace them?
originalSourceURL = hookSourceAndMetadata.runtimeSourceURL;
}

if (__DEBUG__) {
console.log(
`parseSourceAST() mapped line ${lineNumber}->${originalSourceLineNumber} and column ${columnNumber}->${originalSourceColumnNumber}`,
);
}

hookParsedMetadata.originalSourceCode = originalSourceCode;
hookParsedMetadata.originalSourceURL = originalSourceURL;
hookParsedMetadata.originalSourceLineNumber = originalSourceLineNumber;
hookParsedMetadata.originalSourceColumnNumber = originalSourceColumnNumber;

if (hasHookMap) {
if (__DEBUG__) {
console.log(
`parseSourceAST() Found hookMap and skipping parsing for "${originalSourceURL}"`,
);
}
// If there's a hook map present from an extended sourcemap then
// we don't need to parse the source files and instead can use the
// hook map to extract hook names.
return;
}

if (__DEBUG__) {
console.log(
`parseSourceAST() Did not find hook map for "${originalSourceURL}"`,
);
}

// The cache also serves to deduplicate parsing by URL in our loop over location keys.
// This may need to change if we switch to async parsing.
const sourceMetadata = originalURLToMetadataCache.get(originalSourceURL);
Expand Down