Skip to content
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

lib,src: extract sourceMappingURL from module #51690

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,12 @@ async function importModuleDynamically(specifier, { url }, attributes) {
translators.set('module', async function moduleStrategy(url, source, isMain) {
assertBufferSource(source, true, 'load');
source = stringify(source);
maybeCacheSourceMap(url, source);
debug(`Translating StandardModule ${url}`);
const module = new ModuleWrap(url, undefined, source, 0, 0);
// Cache the source map for the module if present.
if (module.sourceMapURL) {
maybeCacheSourceMap(url, source, null, false, undefined, module.sourceMapURL);
}
const { registerModule } = require('internal/modules/esm/utils');
registerModule(module, {
__proto__: null,
Expand Down Expand Up @@ -206,11 +209,11 @@ function enrichCJSError(err, content, filename) {
* @param {string} filename - The filename of the module.
*/
function loadCJSModule(module, source, url, filename) {
let compiledWrapper;
let compileResult;
const hostDefinedOptionId = vm_dynamic_import_default_internal;
const importModuleDynamically = vm_dynamic_import_default_internal;
try {
compiledWrapper = internalCompileFunction(
compileResult = internalCompileFunction(
source, // code,
filename, // filename
0, // lineOffset
Expand All @@ -228,11 +231,17 @@ function loadCJSModule(module, source, url, filename) {
],
hostDefinedOptionId, // hostDefinedOptionsId
importModuleDynamically, // importModuleDynamically
).function;
);
} catch (err) {
enrichCJSError(err, source, filename);
throw err;
}
// Cache the source map for the cjs module if present.
if (compileResult.sourceMapURL) {
maybeCacheSourceMap(url, source, null, false, undefined, compileResult.sourceMapURL);
}

const compiledWrapper = compileResult.function;

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

maybeCacheSourceMap(url, source);

const { exportNames, module } = cjsPreparseModuleExports(filename, source);
cjsCache.set(url, module);
const namesWithDefault = exportNames.has('default') ?
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
return;
}

// FIXME: callers should obtain sourceURL from v8 and pass it
// rather than leaving it undefined and extract by regex.
if (sourceURL === undefined) {
sourceURL = extractSourceURLMagicComment(content);
}
Expand Down
7 changes: 7 additions & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
try_catch.ReThrow();
return;
}

if (that->Set(context,
realm->env()->source_map_url_string(),
module->GetUnboundModuleScript()->GetSourceMappingURL())
.IsNothing()) {
return;
}
}
}

Expand Down
96 changes: 96 additions & 0 deletions test/es-module/test-esm-source-map.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { spawnPromisified } from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import assert from 'node:assert';
import { execPath } from 'node:process';
import { describe, it } from 'node:test';

describe('esm source-map', { concurrency: true }, () => {
// Issue: https://github.com/nodejs/node/issues/51522

[
[
'in middle from esm',
'source-map/extract-url/esm-url-in-middle.mjs',
true,
],
[
'inside string from esm',
'source-map/extract-url/esm-url-in-string.mjs',
false,
],
].forEach(([name, path, shouldExtract]) => {
it((shouldExtract ? 'should extract source map url' : 'should not extract source map url') + name, async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--enable-source-maps',
fixtures.path(path),
]);

assert.strictEqual(stdout, '');
if (shouldExtract) {
assert.match(stderr, /index\.ts/);
} else {
assert.doesNotMatch(stderr, /index\.ts/);
}
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});
});

[
[
'in middle from esm imported by esm',
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/esm-url-in-middle.mjs'))}`,
true,
],
[
'in middle from cjs imported by esm',
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/cjs-url-in-middle.js'))}`,
true,
],
[
'in middle from cjs required by esm',
"import { createRequire } from 'module';" +
'const require = createRequire(import.meta.url);' +
`require(${JSON.stringify(fixtures.path('source-map/extract-url/cjs-url-in-middle.js'))})`,
true,
],

[
'inside string from esm imported by esm',
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/esm-url-in-string.mjs'))}`,
false,
],
[
'inside string from cjs imported by esm',
`import ${JSON.stringify(fixtures.fileURL('source-map/extract-url/cjs-url-in-string.js'))}`,
false,
],
[
'inside string from cjs required by esm',
"import { createRequire } from 'module';" +
'const require = createRequire(import.meta.url);' +
`require(${JSON.stringify(fixtures.path('source-map/extract-url/cjs-url-in-string.js'))})`,
false,
],
].forEach(([name, evalCode, shouldExtract]) => {
it((shouldExtract ? 'should extract source map url' : 'should not extract source map url') + name, async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--enable-source-maps',
'--input-type=module',
'--eval',
evalCode,
]);

assert.strictEqual(stdout, '');
if (shouldExtract) {
assert.match(stderr, /index\.ts/);
} else {
assert.doesNotMatch(stderr, /index\.ts/);
}
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});
});
});
5 changes: 5 additions & 0 deletions test/fixtures/source-map/extract-url/cjs-url-in-middle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

throw new Error("Hello world!");
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
console.log(1);
//
5 changes: 5 additions & 0 deletions test/fixtures/source-map/extract-url/cjs-url-in-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

throw new Error("Hello world!");`
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
console.log(1);
//`
5 changes: 5 additions & 0 deletions test/fixtures/source-map/extract-url/esm-url-in-middle.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

throw new Error("Hello world!");
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
console.log(1);
//
5 changes: 5 additions & 0 deletions test/fixtures/source-map/extract-url/esm-url-in-string.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

throw new Error("Hello world!");`
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
console.log(1);
//`