module: remove bogus assertion in CJS entrypoint handling with --import

The synchronous CJS translator can handle entrypoints now, this can be hit when --import is used, so lift the bogus assertions and added tests. PR-URL: nodejs#54592 Fixes: nodejs#54577 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
IlyasShabi · Sep 17, 2024 · 3c4ef34 · 3c4ef34
1 parent d17fefc
commit 3c4ef34
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 56 deletions.
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -240,11 +240,9 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
 
 translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) {
   initCJSParseSync();
-  assert(!isMain);  // This is only used by imported CJS modules.
 
   return createCJSModuleWrap(url, source, isMain, (module, source, url, filename, isMain) => {
     assert(module === CJSModule._cache[filename]);
-    assert(!isMain);
     wrapModuleLoad(filename, null, isMain);
   });
 });

diff --git a/test/es-module/test-require-module-preload.js b/test/es-module/test-require-module-preload.js
@@ -1,70 +1,117 @@
 'use strict';
 
 require('../common');
-const { spawnSyncAndExitWithoutError } = require('../common/child_process');
-const fixtures = require('../common/fixtures');
-
+const { spawnSyncAndAssert } = require('../common/child_process');
+const { fixturesDir } = require('../common/fixtures');
 const stderr = /ExperimentalWarning: Support for loading ES Module in require/;
 
-// Test named exports.
-{
-  spawnSyncAndExitWithoutError(
-    process.execPath,
-    [ '--experimental-require-module', '-r', fixtures.path('../fixtures/es-module-loaders/module-named-exports.mjs') ],
-    {
-      stderr,
-    }
-  );
-}
+function testPreload(preloadFlag) {
+  // Test named exports.
+  {
+    spawnSyncAndAssert(
+      process.execPath,
+      [
+        '--experimental-require-module',
+        preloadFlag,
+        './es-module-loaders/module-named-exports.mjs',
+        './printA.js',
+      ],
+      {
+        cwd: fixturesDir
+      },
+      {
+        stdout: 'A',
+        stderr,
+        trim: true,
+      }
+    );
+  }
 
-// Test ESM that import ESM.
-{
-  spawnSyncAndExitWithoutError(
-    process.execPath,
-    [ '--experimental-require-module', '-r', fixtures.path('../fixtures/es-modules/import-esm.mjs') ],
-    {
-      stderr,
-      stdout: 'world',
-      trim: true,
-    }
-  );
-}
+  // Test ESM that import ESM.
+  {
+    spawnSyncAndAssert(
+      process.execPath,
+      [
+        '--experimental-require-module',
+        preloadFlag,
+        './es-modules/import-esm.mjs',
+        './printA.js',
+      ],
+      {
+        cwd: fixturesDir
+      },
+      {
+        stderr,
+        stdout: /^world\s+A$/,
+        trim: true,
+      }
+    );
+  }
 
-// Test ESM that import CJS.
-{
-  spawnSyncAndExitWithoutError(
-    process.execPath,
-    [ '--experimental-require-module', '-r', fixtures.path('../fixtures/es-modules/cjs-exports.mjs') ],
-    {
-      stdout: 'ok',
-      stderr,
-      trim: true,
-    }
-  );
-}
+  // Test ESM that import CJS.
+  {
+    spawnSyncAndAssert(
+      process.execPath,
+      [
+        '--experimental-require-module',
+        preloadFlag,
+        './es-modules/cjs-exports.mjs',
+        './printA.js',
+      ],
+      {
+        cwd: fixturesDir
+      },
+      {
+        stdout: /^ok\s+A$/,
+        stderr,
+        trim: true,
+      }
+    );
+  }
 
-// Test ESM that require() CJS.
-// Can't use the common/index.mjs here because that checks the globals, and
-// -r injects a bunch of globals.
-{
-  spawnSyncAndExitWithoutError(
-    process.execPath,
-    [ '--experimental-require-module', '-r', fixtures.path('../fixtures/es-modules/require-cjs.mjs') ],
-    {
-      stdout: 'world',
-      stderr,
-      trim: true,
-    }
-  );
+  // Test ESM that require() CJS.
+  // Can't use the common/index.mjs here because that checks the globals, and
+  // -r injects a bunch of globals.
+  {
+    spawnSyncAndAssert(
+      process.execPath,
+      [
+        '--experimental-require-module',
+        preloadFlag,
+        './es-modules/require-cjs.mjs',
+        './printA.js',
+      ],
+      {
+        cwd: fixturesDir
+      },
+      {
+        stdout: /^world\s+A$/,
+        stderr,
+        trim: true,
+      }
+    );
+  }
 }
 
-// Test "type": "module" and "main" field in package.json.
+testPreload('--require');
+testPreload('--import');
+
+// Test "type": "module" and "main" field in package.json, this is only for --require because
+// --import does not support extension-less preloads.
 {
-  spawnSyncAndExitWithoutError(
+  spawnSyncAndAssert(
     process.execPath,
-    [ '--experimental-require-module', '-r', fixtures.path('../fixtures/es-modules/package-type-module') ],
+    [
+      '--experimental-require-module',
+      '--require',
+      './es-modules/package-type-module',
+      './printA.js',
+    ],
+    {
+      cwd: fixturesDir
+    },
     {
-      stdout: 'package-type-module',
+      stdout: /^package-type-module\s+A$/,
       stderr,
       trim: true,
     }