Skip to content

Commit a0acfc2

Browse files
committed
src,lib: obtain sourceURL in magic comments from V8
Obtain sourceURL magic comments via V8 API to avoid second round of extraction of magic comments. Updates source map snapshot normalization to allow testing full path names inside the test outputs.
1 parent 38757c9 commit a0acfc2

31 files changed

+195
-136
lines changed

lib/internal/main/embedding.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ function embedderRunCjs(content) {
5656
function: compiledWrapper,
5757
cachedDataRejected,
5858
sourceMapURL,
59+
sourceURL,
5960
} = compileFunctionForCJSLoader(
6061
content,
6162
filename,
@@ -69,7 +70,7 @@ function embedderRunCjs(content) {
6970
content,
7071
customModule,
7172
false, // isGeneratedSource
72-
undefined, // sourceURL, TODO(joyeecheung): should be extracted by V8
73+
sourceURL,
7374
sourceMapURL,
7475
);
7576
}

lib/internal/modules/cjs/loader.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1640,9 +1640,9 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
16401640
);
16411641

16421642
// Cache the source map for the module if present.
1643-
const { sourceMapURL } = script;
1643+
const { sourceMapURL, sourceURL } = script;
16441644
if (sourceMapURL) {
1645-
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, sourceMapURL);
1645+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, sourceURL, sourceMapURL);
16461646
}
16471647

16481648
return {
@@ -1667,7 +1667,7 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
16671667

16681668
// Cache the source map for the module if present.
16691669
if (result.sourceMapURL) {
1670-
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, result.sourceMapURL);
1670+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, result.sourceURL, result.sourceMapURL);
16711671
}
16721672

16731673
return result;

lib/internal/modules/esm/translators.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,10 @@ translators.set('module', function moduleStrategy(url, source, isMain) {
121121
function loadCJSModule(module, source, url, filename, isMain) {
122122
const compileResult = compileFunctionForCJSLoader(source, filename, false /* is_sea_main */, false);
123123

124-
const { function: compiledWrapper, sourceMapURL } = compileResult;
124+
const { function: compiledWrapper, sourceMapURL, sourceURL } = compileResult;
125125
// Cache the source map for the cjs module if present.
126126
if (sourceMapURL) {
127-
maybeCacheSourceMap(url, source, module, false, undefined, sourceMapURL);
127+
maybeCacheSourceMap(url, source, module, false, sourceURL, sourceMapURL);
128128
}
129129

130130
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();

lib/internal/modules/esm/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ function compileSourceTextModule(url, source, cascadedLoader) {
353353
}
354354
// Cache the source map for the module if present.
355355
if (wrap.sourceMapURL) {
356-
maybeCacheSourceMap(url, source, wrap, false, undefined, wrap.sourceMapURL);
356+
maybeCacheSourceMap(url, source, wrap, false, wrap.sourceURL, wrap.sourceMapURL);
357357
}
358358
return wrap;
359359
}

lib/internal/source_map/source_map_cache.js

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ function extractSourceMapURLMagicComment(content) {
139139
}
140140

141141
/**
142-
* Caches the source map if it is present in the content, with the given filename, moduleInstance, and sourceURL.
142+
* Caches the source map, with the given filename, moduleInstance, sourceURL and sourceMapURL.
143+
* This function does not automatically extract the source map from the content. The caller should either
144+
* extract the source map from the content via V8 API or use {@link extractSourceURLMagicComment} explicitly.
145+
*
143146
* @param {string} filename - the actual filename
144147
* @param {string} content - the actual source content
145148
* @param {import('internal/modules/cjs/loader').Module | ModuleWrap} moduleInstance - a module instance that
@@ -162,19 +165,15 @@ function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSourc
162165
return;
163166
}
164167

165-
if (sourceMapURL === undefined) {
166-
sourceMapURL = extractSourceMapURLMagicComment(content);
167-
}
168-
169168
// Bail out when there is no source map url.
170169
if (typeof sourceMapURL !== 'string') {
171170
return;
172171
}
173172

174-
// FIXME: callers should obtain sourceURL from v8 and pass it
175-
// rather than leaving it undefined and extract by regex.
176-
if (sourceURL === undefined) {
177-
sourceURL = extractSourceURLMagicComment(content);
173+
if (sourceURL !== undefined) {
174+
// SourceURL magic comment content might be a file path or URL.
175+
// Normalize the sourceURL to be a file URL if it is a file path.
176+
sourceURL = normalizeReferrerURL(sourceURL);
178177
}
179178

180179
const data = dataFromUrl(filename, sourceMapURL);
@@ -215,8 +214,13 @@ function maybeCacheGeneratedSourceMap(content) {
215214
if (sourceURL === null) {
216215
return;
217216
}
217+
const sourceMapURL = extractSourceMapURLMagicComment(content);
218+
if (sourceMapURL === null) {
219+
return;
220+
}
218221
try {
219-
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL);
222+
// Use the sourceURL as the filename, and do not create a duplicate entry.
223+
maybeCacheSourceMap(sourceURL, content, null, true, undefined /** no duplicated sourceURL */, sourceMapURL);
220224
} catch (err) {
221225
// This can happen if the filename is not a valid URL.
222226
// If we fail to cache the source map, we should not fail the whole process.

src/env_properties.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@
363363
V(sni_context_string, "sni_context") \
364364
V(source_string, "source") \
365365
V(source_map_url_string, "sourceMapURL") \
366+
V(source_url_string, "sourceURL") \
366367
V(specifier_string, "specifier") \
367368
V(stack_string, "stack") \
368369
V(standard_name_string, "standardName") \

src/module_wrap.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
308308
return;
309309
}
310310

311+
if (that->Set(context,
312+
realm->env()->source_url_string(),
313+
module->GetUnboundModuleScript()->GetSourceURL())
314+
.IsNothing()) {
315+
return;
316+
}
317+
311318
if (that->Set(context,
312319
realm->env()->source_map_url_string(),
313320
module->GetUnboundModuleScript()->GetSourceMappingURL())

src/node_contextify.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,6 +1111,13 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
11111111
return;
11121112
}
11131113

1114+
if (args.This()
1115+
->Set(env->context(),
1116+
env->source_url_string(),
1117+
v8_script->GetSourceURL())
1118+
.IsNothing())
1119+
return;
1120+
11141121
if (args.This()
11151122
->Set(env->context(),
11161123
env->source_map_url_string(),
@@ -1582,6 +1589,15 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
15821589
Local<Object> result = Object::New(isolate);
15831590
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
15841591
return Object::New(env->isolate());
1592+
1593+
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
1594+
// present.
1595+
if (result
1596+
->Set(parsing_context,
1597+
env->source_url_string(),
1598+
fn->GetScriptOrigin().ResourceName())
1599+
.IsNothing())
1600+
return Object::New(env->isolate());
15851601
if (result
15861602
->Set(parsing_context,
15871603
env->source_map_url_string(),
@@ -1825,12 +1841,16 @@ static void CompileFunctionForCJSLoader(
18251841
Local<Name> names[] = {
18261842
env->cached_data_rejected_string(),
18271843
env->source_map_url_string(),
1844+
env->source_url_string(),
18281845
env->function_string(),
18291846
FIXED_ONE_BYTE_STRING(isolate, "canParseAsESM"),
18301847
};
18311848
Local<Value> values[] = {
18321849
Boolean::New(isolate, cache_rejected),
18331850
fn.IsEmpty() ? undefined : fn->GetScriptOrigin().SourceMapUrl(),
1851+
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
1852+
// present.
1853+
fn.IsEmpty() ? undefined : fn->GetScriptOrigin().ResourceName(),
18341854
fn.IsEmpty() ? undefined : fn.As<Value>(),
18351855
Boolean::New(isolate, can_parse_as_esm),
18361856
};

test/common/assertSnapshot.js

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ const assert = require('node:assert/strict');
88
const stackFramesRegexp = /(?<=\n)(\s+)((.+?)\s+\()?(?:\(?(.+?):(\d+)(?::(\d+))?)\)?(\s+\{)?(\[\d+m)?(\n|$)/g;
99
const windowNewlineRegexp = /\r/g;
1010

11+
function replaceExperimentalWarning(str) {
12+
return str.replace(/\(node:\d+\) ExperimentalWarning: (.*)\n/g, '')
13+
.replace('(Use `node --trace-warnings ...` to show where the warning was created)\n', '');
14+
}
15+
1116
function replaceNodeVersion(str) {
1217
return str.replaceAll(process.version, '*');
1318
}
@@ -16,6 +21,10 @@ function replaceStackTrace(str, replacement = '$1*$7$8\n') {
1621
return str.replace(stackFramesRegexp, replacement);
1722
}
1823

24+
function replaceInternalStackTrace(str) {
25+
return str.replaceAll(/(\W+).*node:internal.*/g, '$1*');
26+
}
27+
1928
function replaceWindowsLineEndings(str) {
2029
return str.replace(windowNewlineRegexp, '');
2130
}
@@ -24,8 +33,20 @@ function replaceWindowsPaths(str) {
2433
return common.isWindows ? str.replaceAll(path.win32.sep, path.posix.sep) : str;
2534
}
2635

27-
function replaceFullPaths(str) {
28-
return str.replaceAll('\\\'', "'").replaceAll(path.resolve(__dirname, '../..'), '');
36+
function replaceWindowsDriveLetter(str) {
37+
if (!common.isWindows) {
38+
return str;
39+
}
40+
const currentDriveLetter = path.parse(process.cwd()).root.substring(0, 1).toLowerCase();
41+
const regex = new RegExp(`${currentDriveLetter}:`, 'gi');
42+
return str.replaceAll('\\\'', "'").replaceAll(regex, '');
43+
}
44+
45+
function transformCwd(replacement = '') {
46+
const cwd = process.cwd();
47+
return (str) => {
48+
return str.replaceAll('\\\'', "'").replaceAll(cwd, replacement);
49+
};
2950
}
3051

3152
function transform(...args) {
@@ -94,11 +115,14 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
94115
module.exports = {
95116
assertSnapshot,
96117
getSnapshotPath,
97-
replaceFullPaths,
118+
replaceExperimentalWarning,
98119
replaceNodeVersion,
99120
replaceStackTrace,
121+
replaceInternalStackTrace,
100122
replaceWindowsLineEndings,
101123
replaceWindowsPaths,
124+
replaceWindowsDriveLetter,
102125
spawnAndAssert,
103126
transform,
127+
transformCwd,
104128
};
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site-min.js:1:156)
3-
at functionC (*enclosing-call-site-min.js:1:97)
4-
at functionB (*enclosing-call-site-min.js:1:60)
5-
at functionA (*enclosing-call-site-min.js:1:26)
6-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
3+
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
4+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
5+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
6+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
77
Error: an error!
8-
at functionD (*enclosing-call-site.js:16:17)
9-
at functionC (*enclosing-call-site.js:10:3)
10-
at functionB (*enclosing-call-site.js:6:3)
11-
at functionA (*enclosing-call-site.js:2:3)
12-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
8+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
9+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
10+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
11+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
12+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site-min.js:1:156)
3-
at functionC (*enclosing-call-site-min.js:1:97)
4-
at functionB (*enclosing-call-site-min.js:1:60)
5-
at functionA (*enclosing-call-site-min.js:1:26)
6-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
3+
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
4+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
5+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
6+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
77
Error: an error!
8-
at functionD (*enclosing-call-site.js:16:17)
9-
at functionC (*enclosing-call-site.js:10:3)
10-
at functionB (*enclosing-call-site.js:6:3)
11-
at functionA (*enclosing-call-site.js:2:3)
12-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
8+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
9+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
10+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
11+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
12+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site.js:16:17)
3-
at functionC (*enclosing-call-site.js:10:3)
4-
at functionB (*enclosing-call-site.js:6:3)
5-
at functionA (*enclosing-call-site.js:2:3)
6-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
3+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
4+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
5+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
6+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
77
Error: an error!
8-
at functionD (*enclosing-call-site-min.js:1:156)
9-
at functionC (*enclosing-call-site-min.js:1:97)
10-
at functionB (*enclosing-call-site-min.js:1:60)
11-
at functionA (*enclosing-call-site-min.js:1:26)
12-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
8+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
9+
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
10+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
11+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
12+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*node_modules*error-stack*enclosing-call-site.js:16:17)
3-
at functionC (*node_modules*error-stack*enclosing-call-site.js:10:3)
4-
at functionB (*node_modules*error-stack*enclosing-call-site.js:6:3)
5-
at functionA (*node_modules*error-stack*enclosing-call-site.js:2:3)
6-
at Object.<anonymous> (*node_modules*error-stack*enclosing-call-site.js:24:3)
2+
at functionD (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site.js:16:17)
3+
at functionC (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site.js:10:3)
4+
at functionB (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site.js:6:3)
5+
at functionA (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site.js:2:3)
6+
at Object.<anonymous> (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site.js:24:3)
77
Error: an error!
8-
at functionD (*node_modules*error-stack*enclosing-call-site-min.js:1:156)
9-
at functionC (*node_modules*error-stack*enclosing-call-site-min.js:1:97)
10-
at functionB (*node_modules*error-stack*enclosing-call-site-min.js:1:60)
11-
at functionA (*node_modules*error-stack*enclosing-call-site-min.js:1:26)
12-
at Object.<anonymous> (*node_modules*error-stack*enclosing-call-site-min.js:1:199)
8+
at functionD (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site-min.js:1:156)
9+
at functionC (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site-min.js:1:97)
10+
at functionB (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site-min.js:1:60)
11+
at functionA (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site-min.js:1:26)
12+
at Object.<anonymous> (*/test/fixtures/source-map/node_modules/error-stack/enclosing-call-site-min.js:1:199)
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site.js:16:17)
3-
at functionC (*enclosing-call-site.js:10:3)
4-
at functionB (*enclosing-call-site.js:6:3)
5-
at functionA (*enclosing-call-site.js:2:3)
6-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
3+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
4+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
5+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
6+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
77
Error: an error!
8-
at functionD (*enclosing-call-site-min.js:1:156)
9-
at functionC (*enclosing-call-site-min.js:1:97)
10-
at functionB (*enclosing-call-site-min.js:1:60)
11-
at functionA (*enclosing-call-site-min.js:1:26)
12-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
8+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
9+
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
10+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
11+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
12+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
*enclosing-call-site.js:26
1+
*/test/fixtures/source-map/enclosing-call-site.js:26
22
throw err
33
^
44

55

66
Error: an error!
7-
at functionD (*enclosing-call-site.js:16:17)
8-
at functionC (*enclosing-call-site.js:10:3)
9-
at functionB (*enclosing-call-site.js:6:3)
10-
at functionA (*enclosing-call-site.js:2:3)
11-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
7+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
8+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
9+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
10+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
11+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
1212

1313
Node.js *

test/fixtures/source-map/output/source_map_eval.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ Error.stackTraceLimit = 3;
77
const fs = require('fs');
88

99
const content = fs.readFileSync(require.resolve('../tabs.js'), 'utf8');
10+
// SourceURL magic comment is hardcoded in the source content.
1011
eval(content);
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
*tabs.coffee:26
1+
/synthethized/workspace/tabs.coffee:26
22
alert "I knew it!"
33
^
44

55

66
ReferenceError: alert is not defined
7-
at Object.eval (*tabs.coffee:26:2)
8-
at eval (*tabs.coffee:1:14)
9-
at Object.<anonymous> (*output*source_map_eval.js:10:1)
7+
at Object.eval (/synthethized/workspace/tabs.coffee:26:2)
8+
at eval (/synthethized/workspace/tabs.coffee:1:14)
9+
at Object.<anonymous> (*/test/fixtures/source-map/output/source_map_eval.js:11:1)
1010

1111
Node.js *

0 commit comments

Comments
 (0)