Skip to content

Commit 861e040

Browse files
unbyterichardlau
unbyte
authored and
richardlau
committedMar 25, 2024
lib,src: extract sourceMappingURL from module
PR-URL: #51690 Refs: #51522 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
·
v20.19.0v20.12.0
1 parent 9082cc5 commit 861e040

File tree

8 files changed

+138
-6
lines changed

8 files changed

+138
-6
lines changed
 

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

+13-6
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,12 @@ async function importModuleDynamically(specifier, { url }, attributes) {
164164
translators.set('module', async function moduleStrategy(url, source, isMain) {
165165
assertBufferSource(source, true, 'load');
166166
source = stringify(source);
167-
maybeCacheSourceMap(url, source);
168167
debug(`Translating StandardModule ${url}`);
169168
const module = new ModuleWrap(url, undefined, source, 0, 0);
169+
// Cache the source map for the module if present.
170+
if (module.sourceMapURL) {
171+
maybeCacheSourceMap(url, source, null, false, undefined, module.sourceMapURL);
172+
}
170173
const { registerModule } = require('internal/modules/esm/utils');
171174
registerModule(module, {
172175
__proto__: null,
@@ -206,11 +209,11 @@ function enrichCJSError(err, content, filename) {
206209
* @param {string} filename - The filename of the module.
207210
*/
208211
function loadCJSModule(module, source, url, filename) {
209-
let compiledWrapper;
212+
let compileResult;
210213
const hostDefinedOptionId = vm_dynamic_import_default_internal;
211214
const importModuleDynamically = vm_dynamic_import_default_internal;
212215
try {
213-
compiledWrapper = internalCompileFunction(
216+
compileResult = internalCompileFunction(
214217
source, // code,
215218
filename, // filename
216219
0, // lineOffset
@@ -228,11 +231,17 @@ function loadCJSModule(module, source, url, filename) {
228231
],
229232
hostDefinedOptionId, // hostDefinedOptionsId
230233
importModuleDynamically, // importModuleDynamically
231-
).function;
234+
);
232235
} catch (err) {
233236
enrichCJSError(err, source, filename);
234237
throw err;
235238
}
239+
// Cache the source map for the cjs module if present.
240+
if (compileResult.sourceMapURL) {
241+
maybeCacheSourceMap(url, source, null, false, undefined, compileResult.sourceMapURL);
242+
}
243+
244+
const compiledWrapper = compileResult.function;
236245

237246
const __dirname = dirname(filename);
238247
// eslint-disable-next-line func-name-matching,func-style
@@ -290,8 +299,6 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
290299
// In case the source was not provided by the `load` step, we need fetch it now.
291300
source = stringify(source ?? getSource(new URL(url)).source);
292301

293-
maybeCacheSourceMap(url, source);
294-
295302
const { exportNames, module } = cjsPreparseModuleExports(filename, source);
296303
cjsCache.set(url, module);
297304
const namesWithDefault = exportNames.has('default') ?

‎lib/internal/source_map/source_map_cache.js

+2
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
123123
return;
124124
}
125125

126+
// FIXME: callers should obtain sourceURL from v8 and pass it
127+
// rather than leaving it undefined and extract by regex.
126128
if (sourceURL === undefined) {
127129
sourceURL = extractSourceURLMagicComment(content);
128130
}

‎src/module_wrap.cc

+7
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
222222
try_catch.ReThrow();
223223
return;
224224
}
225+
226+
if (that->Set(context,
227+
realm->env()->source_map_url_string(),
228+
module->GetUnboundModuleScript()->GetSourceMappingURL())
229+
.IsNothing()) {
230+
return;
231+
}
225232
}
226233
}
227234

‎test/es-module/test-esm-source-map.mjs

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { spawnPromisified } from '../common/index.mjs';
2+
import * as fixtures from '../common/fixtures.mjs';
3+
import assert from 'node:assert';
4+
import { execPath } from 'node:process';
5+
import { describe, it } from 'node:test';
6+
7+
describe('esm source-map', { concurrency: true }, () => {
8+
// Issue: https://github.com/nodejs/node/issues/51522
9+
10+
[
11+
[
12+
'in middle from esm',
13+
'source-map/extract-url/esm-url-in-middle.mjs',
14+
true,
15+
],
16+
[
17+
'inside string from esm',
18+
'source-map/extract-url/esm-url-in-string.mjs',
19+
false,
20+
],
21+
].forEach(([name, path, shouldExtract]) => {
22+
it((shouldExtract ? 'should extract source map url' : 'should not extract source map url') + name, async () => {
23+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
24+
'--no-warnings',
25+
'--enable-source-maps',
26+
fixtures.path(path),
27+
]);
28+
29+
assert.strictEqual(stdout, '');
30+
if (shouldExtract) {
31+
assert.match(stderr, /index\.ts/);
32+
} else {
33+
assert.doesNotMatch(stderr, /index\.ts/);
34+
}
35+
assert.strictEqual(code, 1);
36+
assert.strictEqual(signal, null);
37+
});
38+
});
39+
40+
[
41+
[
42+
'in middle from esm imported by esm',
43+
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/esm-url-in-middle.mjs'))}`,
44+
true,
45+
],
46+
[
47+
'in middle from cjs imported by esm',
48+
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/cjs-url-in-middle.js'))}`,
49+
true,
50+
],
51+
[
52+
'in middle from cjs required by esm',
53+
"import { createRequire } from 'module';" +
54+
'const require = createRequire(import.meta.url);' +
55+
`require(${JSON.stringify(fixtures.path('source-map/extract-url/cjs-url-in-middle.js'))})`,
56+
true,
57+
],
58+
59+
[
60+
'inside string from esm imported by esm',
61+
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/esm-url-in-string.mjs'))}`,
62+
false,
63+
],
64+
[
65+
'inside string from cjs imported by esm',
66+
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/cjs-url-in-string.js'))}`,
67+
false,
68+
],
69+
[
70+
'inside string from cjs required by esm',
71+
"import { createRequire } from 'module';" +
72+
'const require = createRequire(import.meta.url);' +
73+
`require(${JSON.stringify(fixtures.path('source-map/extract-url/cjs-url-in-string.js'))})`,
74+
false,
75+
],
76+
].forEach(([name, evalCode, shouldExtract]) => {
77+
it((shouldExtract ? 'should extract source map url' : 'should not extract source map url') + name, async () => {
78+
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
79+
'--no-warnings',
80+
'--enable-source-maps',
81+
'--input-type=module',
82+
'--eval',
83+
evalCode,
84+
]);
85+
86+
assert.strictEqual(stdout, '');
87+
if (shouldExtract) {
88+
assert.match(stderr, /index\.ts/);
89+
} else {
90+
assert.doesNotMatch(stderr, /index\.ts/);
91+
}
92+
assert.strictEqual(code, 1);
93+
assert.strictEqual(signal, null);
94+
});
95+
});
96+
});

‎test/fixtures/source-map/extract-url/cjs-url-in-middle.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
throw new Error("Hello world!");
3+
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
4+
console.log(1);
5+
//

‎test/fixtures/source-map/extract-url/cjs-url-in-string.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
throw new Error("Hello world!");`
3+
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
4+
console.log(1);
5+
//`

‎test/fixtures/source-map/extract-url/esm-url-in-middle.mjs

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
throw new Error("Hello world!");
3+
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
4+
console.log(1);
5+
//

‎test/fixtures/source-map/extract-url/esm-url-in-string.mjs

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
throw new Error("Hello world!");`
3+
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
4+
console.log(1);
5+
//`

0 commit comments

Comments
 (0)
Please sign in to comment.