esm: disable non-js exts outside package scopes

PR-URL: #30501 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Myles Borins <[email protected]>
nodejs · Feb 6, 2020 · 2275da5 · 2275da5
1 parent d0f4a2f
commit 2275da5
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 22 deletions.
diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -1310,8 +1310,7 @@ _defaultEnv_ is the conditional environment name priority array,
 >       1. Return _"module"_.
 >    1. Throw an _Unsupported File Extension_ error.
 > 1. Otherwise,
->    1. If _isMain_ is **true** or _url_ ends in _".js"_, _".json"_ or
->       _".node"_, then
+>    1. If _isMain_ is **true**, then
 >       1. Return _"commonjs"_.
 >    1. Throw an _Unsupported File Extension_ error.
 

diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js
@@ -78,9 +78,19 @@ function resolve(specifier, parentURL) {
   }
 
   const isMain = parentURL === undefined;
-  if (isMain)
+  if (isMain) {
     parentURL = pathToFileURL(`${process.cwd()}/`).href;
 
+    // This is the initial entry point to the program, and --input-type has
+    // been passed as an option; but --input-type can only be used with
+    // --eval, --print or STDIN string input. It is not allowed with file
+    // input, to avoid user confusion over how expansive the effect of the
+    // flag should be (i.e. entry point only, package scope surrounding the
+    // entry point, etc.).
+    if (typeFlag)
+      throw new ERR_INPUT_TYPE_NOT_ALLOWED();
+  }
+
   let url = moduleWrapResolve(specifier, parentURL);
 
   if (isMain ? !preserveSymlinksMain : !preserveSymlinks) {
@@ -93,27 +103,13 @@ function resolve(specifier, parentURL) {
     url.hash = old.hash;
   }
 
-  const type = getPackageType(url.href);
-
   const ext = extname(url.pathname);
-  const extMap =
-      type !== TYPE_MODULE ? legacyExtensionFormatMap : extensionFormatMap;
-  let format = extMap[ext];
-
-  if (isMain && typeFlag) {
-    // This is the initial entry point to the program, and --input-type has
-    // been passed as an option; but --input-type can only be used with
-    // --eval, --print or STDIN string input. It is not allowed with file
-    // input, to avoid user confusion over how expansive the effect of the
-    // flag should be (i.e. entry point only, package scope surrounding the
-    // entry point, etc.).
-    throw new ERR_INPUT_TYPE_NOT_ALLOWED();
-  }
+  let format = extensionFormatMap[ext];
+  if (ext === '.js' || (!format && isMain))
+    format = getPackageType(url.href) === TYPE_MODULE ? 'module' : 'commonjs';
   if (!format) {
-    if (isMain)
-      format = type === TYPE_MODULE ? 'module' : 'commonjs';
-    else if (esModuleSpecifierResolution === 'node')
-      format = 'commonjs';
+    if (esModuleSpecifierResolution === 'node')
+      format = legacyExtensionFormatMap[ext];
     else
       throw new ERR_UNKNOWN_FILE_EXTENSION(fileURLToPath(url));
   }

diff --git a/test/es-module/test-esm-non-js.js b/test/es-module/test-esm-non-js.js
@@ -0,0 +1,21 @@
+'use strict';
+
+const common = require('../common');
+const { spawn } = require('child_process');
+const assert = require('assert');
+
+const entry = require.resolve('./test-esm-json.mjs');
+
+// Verify non-js extensions fail for ESM
+const child = spawn(process.execPath, ['--experimental-modules', entry]);
+
+let stderr = '';
+child.stderr.setEncoding('utf8');
+child.stderr.on('data', (data) => {
+  stderr += data;
+});
+child.on('close', common.mustCall((code, signal) => {
+  assert.strictEqual(code, 1);
+  assert.strictEqual(signal, null);
+  assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
+}));