Skip to content

Commit a5df778

Browse files
legendecasaduh95
legendecas
authored and
aduh95
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. PR-URL: #58389 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent 7d505f4 commit a5df778

31 files changed

+261
-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
@@ -1636,9 +1636,9 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
16361636
);
16371637

16381638
// Cache the source map for the module if present.
1639-
const { sourceMapURL } = script;
1639+
const { sourceMapURL, sourceURL } = script;
16401640
if (sourceMapURL) {
1641-
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, sourceMapURL);
1641+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, sourceURL, sourceMapURL);
16421642
}
16431643

16441644
return {
@@ -1663,7 +1663,7 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
16631663

16641664
// Cache the source map for the module if present.
16651665
if (result.sourceMapURL) {
1666-
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, result.sourceMapURL);
1666+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, result.sourceURL, result.sourceMapURL);
16671667
}
16681668

16691669
return result;

‎lib/internal/modules/esm/translators.js

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

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

129129
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
@@ -348,7 +348,7 @@ function compileSourceTextModule(url, source, cascadedLoader) {
348348
}
349349
// Cache the source map for the module if present.
350350
if (wrap.sourceMapURL) {
351-
maybeCacheSourceMap(url, source, wrap, false, undefined, wrap.sourceMapURL);
351+
maybeCacheSourceMap(url, source, wrap, false, wrap.sourceURL, wrap.sourceMapURL);
352352
}
353353
return wrap;
354354
}

‎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
@@ -354,6 +354,7 @@
354354
V(sni_context_string, "sni_context") \
355355
V(source_string, "source") \
356356
V(source_map_url_string, "sourceMapURL") \
357+
V(source_url_string, "sourceURL") \
357358
V(specifier_string, "specifier") \
358359
V(stack_string, "stack") \
359360
V(standard_name_string, "standardName") \

‎src/module_wrap.cc

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

308+
if (that->Set(context,
309+
realm->env()->source_url_string(),
310+
module->GetUnboundModuleScript()->GetSourceURL())
311+
.IsNothing()) {
312+
return;
313+
}
314+
308315
if (that->Set(context,
309316
realm->env()->source_map_url_string(),
310317
module->GetUnboundModuleScript()->GetSourceMappingURL())

‎src/node_contextify.cc

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,13 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
11071107
return;
11081108
}
11091109

1110+
if (args.This()
1111+
->Set(env->context(),
1112+
env->source_url_string(),
1113+
v8_script->GetSourceURL())
1114+
.IsNothing())
1115+
return;
1116+
11101117
if (args.This()
11111118
->Set(env->context(),
11121119
env->source_map_url_string(),
@@ -1565,12 +1572,23 @@ MaybeLocal<Object> ContextifyFunction::CompileFunctionAndCacheResult(
15651572
Local<Object> result = Object::New(isolate);
15661573
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
15671574
return {};
1575+
1576+
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
1577+
// present.
1578+
if (result
1579+
->Set(parsing_context,
1580+
env->source_url_string(),
1581+
fn->GetScriptOrigin().ResourceName())
1582+
.IsNothing()) {
1583+
return {};
1584+
}
15681585
if (result
15691586
->Set(parsing_context,
15701587
env->source_map_url_string(),
15711588
fn->GetScriptOrigin().SourceMapUrl())
1572-
.IsNothing())
1589+
.IsNothing()) {
15731590
return {};
1591+
}
15741592

15751593
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
15761594
if (produce_cached_data) {
@@ -1808,12 +1826,16 @@ static void CompileFunctionForCJSLoader(
18081826
Local<Name> names[] = {
18091827
env->cached_data_rejected_string(),
18101828
env->source_map_url_string(),
1829+
env->source_url_string(),
18111830
env->function_string(),
18121831
FIXED_ONE_BYTE_STRING(isolate, "canParseAsESM"),
18131832
};
18141833
Local<Value> values[] = {
18151834
Boolean::New(isolate, cache_rejected),
18161835
fn.IsEmpty() ? undefined : fn->GetScriptOrigin().SourceMapUrl(),
1836+
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
1837+
// present.
1838+
fn.IsEmpty() ? undefined : fn->GetScriptOrigin().ResourceName(),
18171839
fn.IsEmpty() ? undefined : fn.As<Value>(),
18181840
Boolean::New(isolate, can_parse_as_esm),
18191841
};

‎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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@ Error.stackTraceLimit = 3;
66

77
const fs = require('fs');
88

9-
const content = fs.readFileSync(require.resolve('../tabs.js'), 'utf8');
9+
const content = fs.readFileSync(require.resolve('../tabs-source-url.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+
*/synthesized/workspace/tabs-source-url.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 (*/synthesized/workspace/tabs-source-url.coffee:26:2)
8+
at eval (*/synthesized/workspace/tabs-source-url.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)