module: resolve format for all situations with auto module detection on

Author: dygabo

lib/internal/modules/esm/get_format.js

@@ -92,6 +92,12 @@ let typelessPackageJsonFilesWarnedAbout;
 function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreErrors) {
   const { source } = context;
   const ext = extname(url);
+  const deduceFormat = (fromSource, fromUrl) => {
+    const { existsSync, readFileSync } = require('fs');
+    const realSource = fromSource ?? existsSync(fromUrl) ? readFileSync(fromUrl).toString() : undefined;
+    return realSource ? // Do we have a source? check for module syntax
+      (containsModuleSyntax(realSource, fileURLToPath(fromUrl), fromUrl) ? 'module' : 'commonjs') : 'commonjs';
+  };
 
   if (ext === '.js') {
     const { type: packageType, pjsonPath } = getPackageScopeConfig(url);
@@ -113,9 +119,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
         // `source` is undefined when this is called from `defaultResolve`;
         // but this gets called again from `defaultLoad`/`defaultLoadSync`.
         if (getOptionValue('--experimental-detect-module')) {
-          const format = source ?
-            (containsModuleSyntax(`${source}`, fileURLToPath(url), url) ? 'module' : 'commonjs') :
-            null;
+          const format = deduceFormat(source, url);
           if (format === 'module') {
             // This module has a .js extension, a package.json with no `type` field, and ESM syntax.
             // Warn about the missing `type` field so that the user can avoid the performance penalty of detection.
@@ -155,12 +159,8 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
       }
       default: { // The user did not pass `--experimental-default-type`.
         if (getOptionValue('--experimental-detect-module')) {
-          if (!source) { return null; }
           const format = getFormatOfExtensionlessFile(url);
-          if (format === 'module') {
-            return containsModuleSyntax(`${source}`, fileURLToPath(url), url) ? 'module' : 'commonjs';
-          }
-          return format;
+          return (format === 'module') ? deduceFormat(source, url) : format;
         }
         return 'commonjs';
       }

lib/internal/modules/esm/loader.js

@@ -321,7 +321,7 @@ class ModuleLoader {
    * @returns {ModuleJobBase}
    */
   getModuleJobForRequire(specifier, parentURL, importAttributes) {
-    assert(getOptionValue('--experimental-require-module'));
+    assert(getOptionValue('--experimental-require-module') || getOptionValue('--experimental-detect-module'));
 
     if (canParse(specifier)) {
       const protocol = new URL(specifier).protocol;

src/node_options.h

@@ -110,7 +110,7 @@ class EnvironmentOptions : public Options {
  public:
   bool abort_on_uncaught_exception = false;
   std::vector<std::string> conditions;
-  bool detect_module = false;
+  bool detect_module = true;
   bool print_required_tla = false;
   bool require_module = false;
   std::string dns_result_order;

test/es-module/test-esm-cjs-exports.js

@@ -17,13 +17,17 @@ describe('ESM: importing CJS', { concurrency: !process.env.TEST_PARALLEL }, () =
     assert.strictEqual(stdout, 'ok\n');
   });
 
-  it('should error on invalid CJS exports', async () => {
-    const invalidEntry = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
-    const { code, signal, stderr } = await spawnPromisified(execPath, [invalidEntry]);
+  it('should not error if .mjs imports .js with ESM syntax', async () => {
+    // a .mjs that imports a .js gets automatically correctly resolved by automatic syntax detection
+    // a warning that this detection happened implicitly is being generated.
+    const cjsExportsEntry = fixtures.path('/es-modules/cjs-exports-import.mjs');
+    const { code, signal, stderr, stdout } = await spawnPromisified(execPath, [cjsExportsEntry]);
 
-    assert.strictEqual(code, 1);
+    console.log(`stderr: ${stderr}`);
+    console.log(`stdout: ${stdout}`);
+    assert.strictEqual(code, 0);
     assert.strictEqual(signal, null);
-    assert.ok(stderr.includes('Warning: To load an ES module'));
-    assert.ok(stderr.includes('Unexpected token \'export\''));
+    assert.strictEqual(stdout, '');
+    assert.match(stderr, /[MODULE_TYPELESS_PACKAGE_JSON]/);
   });
 });

test/es-module/test-esm-detect-ambiguous.mjs

@@ -172,7 +172,8 @@ describe('--experimental-detect-module', { concurrency: !process.env.TEST_PARALL
       ]);
 
       strictEqual(stderr, '');
-      strictEqual(stdout, 'null\nexecuted\n');
+      // 'module' format resolved correctly because noext-esm contains ESM syntax
+      strictEqual(stdout, 'module\nexecuted\n');
       strictEqual(code, 0);
       strictEqual(signal, null);
 

test/es-module/test-esm-extensionless-esm-and-wasm.mjs

@@ -61,14 +61,16 @@ describe('extensionless Wasm modules within a "type": "module" package scope', {
 
 describe('extensionless ES modules within no package scope', { concurrency: !process.env.TEST_PARALLEL }, () => {
   // This succeeds with `--experimental-default-type=module`
-  it('should error as the entry point', async () => {
+  it('should not error as the entry point', async () => {
+    // Should not error because detect-module recognizes the ESM syntax
     const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
+      '--no-warnings',
       fixtures.path('es-modules/noext-esm'),
     ]);
 
-    match(stderr, /SyntaxError/);
-    strictEqual(stdout, '');
-    strictEqual(code, 1);
+    strictEqual(stderr, '');
+    match(stdout, /executed/);
+    strictEqual(code, 0);
     strictEqual(signal, null);
   });
 

test/es-module/test-esm-import-flag.mjs

@@ -135,7 +135,7 @@ describe('import modules using --import', { concurrency: !process.env.TEST_PARAL
     assert.strictEqual(signal, null);
   });
 
-  it('should import when running --check fails', async () => {
+  it('should import when running --check succeed', async () => {
     const { code, signal, stderr, stdout } = await spawnPromisified(
       execPath,
       [
@@ -146,9 +146,9 @@ describe('import modules using --import', { concurrency: !process.env.TEST_PARAL
       ]
     );
 
-    assert.match(stderr, /SyntaxError: Unexpected token 'export'/);
+    assert.strictEqual(stderr, '');
     assert.match(stdout, /^\.mjs file\r?\n$/);
-    assert.strictEqual(code, 1);
+    assert.strictEqual(code, 0);
     assert.strictEqual(signal, null);
   });
 

test/es-module/test-esm-resolve-type.mjs

@@ -52,14 +52,12 @@ try {
   /**
    * create a test module and try to resolve it by module name.
    * check the result is as expected
-   *
-   * for test-module-ne: everything .js that is not 'module' is 'commonjs'
    */
   for (const [ moduleName, moduleExtenstion, moduleType, expectedResolvedType ] of
     [ [ 'test-module-mainjs', 'js', 'module', 'module'],
       [ 'test-module-mainmjs', 'mjs', 'module', 'module'],
       [ 'test-module-cjs', 'js', 'commonjs', 'commonjs'],
-      [ 'test-module-ne', 'js', undefined, 'commonjs'],
+      [ 'test-module-ne', 'js', undefined, 'module'], // Resolves to module because syntax was recognized as esm
     ]) {
     process.chdir(previousCwd);
     tmpdir.refresh();

test/es-module/test-require-module-implicit.js

@@ -2,22 +2,14 @@
 'use strict';
 
 // Tests that require()ing modules without explicit module type information
-// warns and errors.
+// does not warn or error. It works instead.
 require('../common');
 const assert = require('assert');
 const { isModuleNamespaceObject } = require('util/types');
+const { strictEqual } = require('node:assert');
 
-assert.throws(() => {
-  require('../fixtures/es-modules/package-without-type/noext-esm');
-}, {
-  message: /Unexpected token 'export'/
-});
-
-assert.throws(() => {
-  require('../fixtures/es-modules/loose.js');
-}, {
-  message: /Unexpected token 'export'/
-});
+strictEqual(require('../fixtures/es-modules/package-without-type/noext-esm').default, 'module');
+strictEqual(require('../fixtures/es-modules/loose.js').default, 'module');
 
 {
   // .mjs should not be matched as default extensions.

test/fixtures/es-modules/cjs-exports-import.mjs

@@ -0,0 +1,2 @@
+import * as mjs_from_js from './invalid-cjs.js';
+export default mjs_from_js;