lib,src: extract sourceMappingURL from module

PR-URL: #51690 Refs: #51522 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs · Feb 26, 2024 · fc0f2cf · fc0f2cf
1 parent 9ac98b1
commit fc0f2cf
Show file tree

Hide file tree

Showing 8 changed files with 138 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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;
+      }
     }
   }
 

diff --git a/test/es-module/test-esm-source-map.mjs 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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,5 @@
+
+throw new Error("Hello world!");`
+//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsiLi4vcHJvamVjdC9pbmRleC50cyJdLAogICJzb3VyY2VzQ29udGVudCI6IFsidGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIpXG4iXSwKICAibWFwcGluZ3MiOiAiO0FBQUEsTUFBTSxJQUFJLE1BQU0sY0FBYzsiLAogICJuYW1lcyI6IFtdCn0K
+console.log(1);
+//`