lib,src: extract sourceMappingURL from module

unbyte · marco-ippolito · commit 8dc3f91eb46d · 2024-02-27T12:10:14.000+01:00
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>
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
@@ -223,6 +223,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);
+//`

Original file line number	Diff line number	Diff line change
`@@ -123,6 +123,8 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo`
`123`	`123`	`return;`
`124`	`124`	`}`
`125`	`125`
	`126`	`+ // FIXME: callers should obtain sourceURL from v8 and pass it`
	`127`	`+ // rather than leaving it undefined and extract by regex.`
`126`	`128`	`if (sourceURL === undefined) {`
`127`	`129`	`sourceURL = extractSourceURLMagicComment(content);`
`128`	`130`	`}`
Original file line number	Diff line number	Diff line change
`@@ -223,6 +223,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {`
`223`	`223`	`try_catch.ReThrow();`
`224`	`224`	`return;`
`225`	`225`	`}`
	`226`	`+`
	`227`	`+ if (that->Set(context,`
	`228`	`+ realm->env()->source_map_url_string(),`
	`229`	`+ module->GetUnboundModuleScript()->GetSourceMappingURL())`
	`230`	`+ .IsNothing()) {`
	`231`	`+ return;`
	`232`	`+ }`
`226`	`233`	`}`
`227`	`234`	`}`
`228`	`235`