-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
src,lib: print source map error source on demand #43875
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
'use strict'; | ||
|
||
const common = require('../common.js'); | ||
const modPath = require.resolve('../fixtures/simple-error-stack.js'); | ||
|
||
const bench = common.createBenchmark(main, { | ||
method: ['without-sourcemap', 'sourcemap'], | ||
n: [1e5], | ||
}); | ||
|
||
function runN(n) { | ||
delete require.cache[modPath]; | ||
const mod = require(modPath); | ||
bench.start(); | ||
for (let i = 0; i < n; i++) { | ||
mod.simpleErrorStack(); | ||
} | ||
bench.end(n); | ||
} | ||
|
||
function main({ n, method }) { | ||
switch (method) { | ||
case 'without-sourcemap': | ||
process.setSourceMapsEnabled(false); | ||
runN(n); | ||
break; | ||
case 'sourcemap': | ||
process.setSourceMapsEnabled(true); | ||
runN(n); | ||
break; | ||
default: | ||
throw new Error(`Unexpected method "${method}"`); | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
'use strict'; | ||
|
||
// Compile with `tsc --inlineSourceMap benchmark/fixtures/simple-error-stack.ts`. | ||
|
||
const lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'; | ||
|
||
function simpleErrorStack() { | ||
try { | ||
(lorem as any).BANG(); | ||
} catch (e) { | ||
return e.stack; | ||
} | ||
} | ||
|
||
export { | ||
simpleErrorStack, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,8 @@ const esmSourceMapCache = new SafeMap(); | |
// The generated sources is not mutable, so we can use a Map without memory concerns: | ||
const generatedSourceMapCache = new SafeMap(); | ||
const kLeadingProtocol = /^\w+:\/\//; | ||
const kSourceMappingURLMagicComment = /\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/; | ||
const kSourceURLMagicComment = /\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/; | ||
|
||
const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); | ||
let SourceMap; | ||
|
@@ -77,7 +79,22 @@ function setSourceMapsEnabled(val) { | |
sourceMapsEnabled = val; | ||
} | ||
|
||
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource) { | ||
function extractSourceURLMagicComment(content) { | ||
const matchSourceURL = RegExpPrototypeExec( | ||
kSourceURLMagicComment, | ||
content | ||
); | ||
if (matchSourceURL === null) { | ||
return null; | ||
} | ||
let sourceURL = matchSourceURL.groups.sourceURL; | ||
if (sourceURL != null && RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { | ||
sourceURL = pathToFileURL(sourceURL).href; | ||
} | ||
return sourceURL; | ||
} | ||
|
||
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL) { | ||
const sourceMapsEnabled = getSourceMapsEnabled(); | ||
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; | ||
try { | ||
|
@@ -87,10 +104,10 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo | |
debug(err); | ||
return; | ||
} | ||
const match = RegExpPrototypeExec( | ||
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/, | ||
content, | ||
); | ||
const match = RegExpPrototypeExec(kSourceMappingURLMagicComment, content); | ||
if (sourceURL === undefined) { | ||
sourceURL = extractSourceURLMagicComment(content); | ||
} | ||
if (match) { | ||
const data = dataFromUrl(filename, match.groups.sourceMappingURL); | ||
const url = data ? null : match.groups.sourceMappingURL; | ||
|
@@ -99,22 +116,33 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo | |
filename, | ||
lineLengths: lineLengths(content), | ||
data, | ||
url | ||
url, | ||
sourceURL, | ||
}); | ||
} else if (isGeneratedSource) { | ||
generatedSourceMapCache.set(filename, { | ||
const entry = { | ||
lineLengths: lineLengths(content), | ||
data, | ||
url | ||
}); | ||
url, | ||
sourceURL | ||
}; | ||
generatedSourceMapCache.set(filename, entry); | ||
if (sourceURL) { | ||
generatedSourceMapCache.set(sourceURL, entry); | ||
} | ||
} else { | ||
// If there is no cjsModuleInstance and is not generated source assume we are in a | ||
// "modules/esm" context. | ||
esmSourceMapCache.set(filename, { | ||
const entry = { | ||
lineLengths: lineLengths(content), | ||
data, | ||
url | ||
}); | ||
url, | ||
sourceURL, | ||
}; | ||
esmSourceMapCache.set(filename, entry); | ||
if (sourceURL) { | ||
esmSourceMapCache.set(sourceURL, entry); | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -123,19 +151,12 @@ function maybeCacheGeneratedSourceMap(content) { | |
const sourceMapsEnabled = getSourceMapsEnabled(); | ||
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; | ||
|
||
const matchSourceURL = RegExpPrototypeExec( | ||
/\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/, | ||
content | ||
); | ||
if (matchSourceURL == null) { | ||
const sourceURL = extractSourceURLMagicComment(content); | ||
if (sourceURL === null) { | ||
return; | ||
} | ||
let sourceURL = matchSourceURL.groups.sourceURL; | ||
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { | ||
sourceURL = pathToFileURL(sourceURL).href; | ||
} | ||
try { | ||
maybeCacheSourceMap(sourceURL, content, null, true); | ||
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL); | ||
} catch (err) { | ||
// This can happen if the filename is not a valid URL. | ||
// If we fail to cache the source map, we should not fail the whole process. | ||
|
@@ -254,33 +275,29 @@ function appendCJSCache(obj) { | |
} | ||
} | ||
|
||
function findSourceMap(sourceURL, isGenerated) { | ||
function findSourceMap(sourceURL) { | ||
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) { | ||
sourceURL = pathToFileURL(sourceURL).href; | ||
} | ||
if (!SourceMap) { | ||
SourceMap = require('internal/source_map/source_map').SourceMap; | ||
} | ||
let sourceMap; | ||
if (isGenerated) { | ||
sourceMap = generatedSourceMapCache.get(sourceURL); | ||
} else { | ||
sourceMap = esmSourceMapCache.get(sourceURL); | ||
if (sourceMap === undefined) { | ||
for (const value of cjsSourceMapCache) { | ||
const filename = ObjectGetValueSafe(value, 'filename'); | ||
if (sourceURL === filename) { | ||
sourceMap = { | ||
data: ObjectGetValueSafe(value, 'data') | ||
}; | ||
} | ||
let sourceMap = esmSourceMapCache.get(sourceURL) ?? generatedSourceMapCache.get(sourceURL); | ||
if (sourceMap === undefined) { | ||
for (const value of cjsSourceMapCache) { | ||
const filename = ObjectGetValueSafe(value, 'filename'); | ||
const cachedSourceURL = ObjectGetValueSafe(value, 'sourceURL'); | ||
if (sourceURL === filename || sourceURL === cachedSourceURL) { | ||
sourceMap = { | ||
data: ObjectGetValueSafe(value, 'data') | ||
}; | ||
} | ||
} | ||
} | ||
if (sourceMap && sourceMap.data) { | ||
return new SourceMap(sourceMap.data); | ||
} | ||
return undefined; | ||
return null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this a breaking change? AVA for example relies on this that it returns undefined here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you open a separate issue? This looks like something unintended that we can fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
module.exports = { | ||
|
Uh oh!
There was an error while loading. Please reload this page.