Skip to content

Commit c8f2eba

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

31 files changed

+174
-140
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: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ 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.
143145
* @param {string} filename - the actual filename
144146
* @param {string} content - the actual source content
145147
* @param {import('internal/modules/cjs/loader').Module | ModuleWrap} moduleInstance - a module instance that
@@ -162,20 +164,13 @@ function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSourc
162164
return;
163165
}
164166

165-
if (sourceMapURL === undefined) {
166-
sourceMapURL = extractSourceMapURLMagicComment(content);
167-
}
168-
169167
// Bail out when there is no source map url.
170168
if (typeof sourceMapURL !== 'string') {
171169
return;
172170
}
173171

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);
178-
}
172+
// Normalize the sourceURL to a file URL if it is a path.
173+
sourceURL = normalizeReferrerURL(sourceURL);
179174

180175
const data = dataFromUrl(filename, sourceMapURL);
181176
// `data` could be null if the source map is invalid.
@@ -192,9 +187,6 @@ function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSourc
192187

193188
if (isGeneratedSource) {
194189
generatedSourceMapCache.set(filename, entry);
195-
if (sourceURL) {
196-
generatedSourceMapCache.set(sourceURL, entry);
197-
}
198190
return;
199191
}
200192
// If it is not a generated source, we assume we are in a "cjs/esm"
@@ -215,8 +207,14 @@ function maybeCacheGeneratedSourceMap(content) {
215207
if (sourceURL === null) {
216208
return;
217209
}
210+
const sourceMapURL = extractSourceMapURLMagicComment(content);
211+
if (sourceMapURL === null) {
212+
return;
213+
}
214+
218215
try {
219-
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL);
216+
// Use the sourceURL as the filename, and do not create a duplicate entry.
217+
maybeCacheSourceMap(sourceURL, content, null, true, undefined /** no duplicated sourceURL */, sourceMapURL);
220218
} catch (err) {
221219
// This can happen if the filename is not a valid URL.
222220
// 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: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ function replaceStackTrace(str, replacement = '$1*$7$8\n') {
1616
return str.replace(stackFramesRegexp, replacement);
1717
}
1818

19+
function replaceInternalStackTrace(str) {
20+
return str.replaceAll(/(\W+).*node:internal.*/g, '$1*');
21+
}
22+
1923
function replaceWindowsLineEndings(str) {
2024
return str.replace(windowNewlineRegexp, '');
2125
}
@@ -24,8 +28,11 @@ function replaceWindowsPaths(str) {
2428
return common.isWindows ? str.replaceAll(path.win32.sep, path.posix.sep) : str;
2529
}
2630

27-
function replaceFullPaths(str) {
28-
return str.replaceAll('\\\'', "'").replaceAll(path.resolve(__dirname, '../..'), '');
31+
function transformProjectRoot(replacement = '') {
32+
const projectRoot = path.resolve(__dirname, '../..');
33+
return (str) => {
34+
return str.replaceAll('\\\'', "'").replaceAll(projectRoot, replacement);
35+
};
2936
}
3037

3138
function transform(...args) {
@@ -94,11 +101,12 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
94101
module.exports = {
95102
assertSnapshot,
96103
getSnapshotPath,
97-
replaceFullPaths,
98104
replaceNodeVersion,
99105
replaceStackTrace,
106+
replaceInternalStackTrace,
100107
replaceWindowsLineEndings,
101108
replaceWindowsPaths,
102109
spawnAndAssert,
103110
transform,
111+
transformProjectRoot,
104112
};

‎test/fixtures/source-map/output/source_map_disabled_by_api.snapshot

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)

‎test/fixtures/source-map/output/source_map_disabled_by_process_api.snapshot

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)

‎test/fixtures/source-map/output/source_map_enabled_by_api.snapshot

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)

‎test/fixtures/source-map/output/source_map_enabled_by_api_node_modules.snapshot

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)

‎test/fixtures/source-map/output/source_map_enabled_by_process_api.snapshot

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)

‎test/fixtures/source-map/output/source_map_enclosing_function.snapshot

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);

‎test/fixtures/source-map/output/source_map_eval.snapshot

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
*tabs.coffee:26
1+
custom:///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 (custom:///synthethized/workspace/tabs.coffee:26:2)
8+
at eval (custom:///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 *

‎test/fixtures/source-map/output/source_map_no_source_file.snapshot

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
*no-source.js:2
1+
*/test/fixtures/source-map/no-source.js:2
22
throw new Error('foo');
33
^
44

55
Error: foo
6-
at Throw (*file-not-exists.ts:2:9)
7-
at Object.<anonymous> (*file-not-exists.ts:5:1)
6+
at Throw (*/test/fixtures/source-map/file-not-exists.ts:2:9)
7+
at Object.<anonymous> (*/test/fixtures/source-map/file-not-exists.ts:5:1)
88

99
Node.js *

0 commit comments

Comments
 (0)