From 861e040b40571e1146e1bcb42d4844a1f92234d9 Mon Sep 17 00:00:00 2001 From: unbyte Date: Tue, 6 Feb 2024 23:44:59 +0800 Subject: [PATCH] lib,src: extract sourceMappingURL from module PR-URL: https://github.com/nodejs/node/pull/51690 Refs: https://github.com/nodejs/node/issues/51522 Reviewed-By: Antoine du Hamel Reviewed-By: Chengzhong Wu Reviewed-By: Joyee Cheung --- lib/internal/modules/esm/translators.js | 19 ++-- lib/internal/source_map/source_map_cache.js | 2 + src/module_wrap.cc | 7 ++ test/es-module/test-esm-source-map.mjs | 96 +++++++++++++++++++ .../extract-url/cjs-url-in-middle.js | 5 + .../extract-url/cjs-url-in-string.js | 5 + .../extract-url/esm-url-in-middle.mjs | 5 + .../extract-url/esm-url-in-string.mjs | 5 + 8 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 test/es-module/test-esm-source-map.mjs create mode 100644 test/fixtures/source-map/extract-url/cjs-url-in-middle.js create mode 100644 test/fixtures/source-map/extract-url/cjs-url-in-string.js create mode 100644 test/fixtures/source-map/extract-url/esm-url-in-middle.mjs create mode 100644 test/fixtures/source-map/extract-url/esm-url-in-string.mjs diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 1268e6adec9435..ca547699d00ed1 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -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, @@ -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 @@ -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 @@ -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') ? diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index fb411487336c29..53c3374fc09176 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -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); } diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 57960b71b73302..501e4d7b7ea180 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -222,6 +222,13 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { try_catch.ReThrow(); return; } + + if (that->Set(context, + realm->env()->source_map_url_string(), + module->GetUnboundModuleScript()->GetSourceMappingURL()) + .IsNothing()) { + return; + } } } diff --git a/test/es-module/test-esm-source-map.mjs b/test/es-module/test-esm-source-map.mjs new file mode 100644 index 00000000000000..b3a41251768675 --- /dev/null +++ b/test/es-module/test-esm-source-map.mjs @@ -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); + }); + }); +}); diff --git a/test/fixtures/source-map/extract-url/cjs-url-in-middle.js b/test/fixtures/source-map/extract-url/cjs-url-in-middle.js new file mode 100644 index 00000000000000..62f1ce898c8307 --- /dev/null +++ b/test/fixtures/source-map/extract-url/cjs-url-in-middle.js @@ -0,0 +1,5 @@ + +throw new Error("Hello world!"); +//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K +console.log(1); +// diff --git a/test/fixtures/source-map/extract-url/cjs-url-in-string.js b/test/fixtures/source-map/extract-url/cjs-url-in-string.js new file mode 100644 index 00000000000000..fcbe35e8788383 --- /dev/null +++ b/test/fixtures/source-map/extract-url/cjs-url-in-string.js @@ -0,0 +1,5 @@ + +throw new Error("Hello world!");` +//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K +console.log(1); +//` diff --git a/test/fixtures/source-map/extract-url/esm-url-in-middle.mjs b/test/fixtures/source-map/extract-url/esm-url-in-middle.mjs new file mode 100644 index 00000000000000..62f1ce898c8307 --- /dev/null +++ b/test/fixtures/source-map/extract-url/esm-url-in-middle.mjs @@ -0,0 +1,5 @@ + +throw new Error("Hello world!"); +//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K +console.log(1); +// diff --git a/test/fixtures/source-map/extract-url/esm-url-in-string.mjs b/test/fixtures/source-map/extract-url/esm-url-in-string.mjs new file mode 100644 index 00000000000000..fcbe35e8788383 --- /dev/null +++ b/test/fixtures/source-map/extract-url/esm-url-in-string.mjs @@ -0,0 +1,5 @@ + +throw new Error("Hello world!");` +//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K +console.log(1); +//`