module: drop support for extensionless main entry points in esm

GeoffreyBooth · codebytere · commit aa266628ba83 · 2020-02-17T10:57:11.000-08:00
PR-URL: #31415 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -33,8 +33,7 @@ initial input, or when referenced by `import` statements within ES module code:
 
 * Files ending in `.mjs`.
 
-* Files ending in `.js`, or extensionless files when run as main entry points on
-  the command line, when the nearest parent `package.json` file contains a
+* Files ending in `.js` when the nearest parent `package.json` file contains a
   top-level field `"type"` with a value of `"module"`.
 
 * Strings passed in as an argument to `--eval` or `--print`, or piped to
@@ -50,18 +49,17 @@ or when referenced by `import` statements within ES module code:
 
 * Files ending in `.cjs`.
 
-* Files ending in `.js`, or extensionless files when run as main entry points on
-  the command line, when the nearest parent `package.json` file contains a
+* Files ending in `.js` when the nearest parent `package.json` file contains a
   top-level field `"type"` with a value of `"commonjs"`.
 
 * Strings passed in as an argument to `--eval` or `--print`, or piped to
   `node` via `STDIN`, with the flag `--input-type=commonjs`.
 
 ### `package.json` `"type"` field
 
-Files ending with `.js` or lacking any extension will be loaded as ES modules
-when the nearest parent `package.json` file contains a top-level field `"type"`
-with a value of `"module"`.
+Files ending with `.js` will be loaded as ES modules when the nearest parent
+`package.json` file contains a top-level field `"type"` with a value of
+`"module"`.
 
 The nearest parent `package.json` is defined as the first `package.json` found
 when searching in the current folder, that folder’s parent, and so on up
@@ -81,14 +79,12 @@ node my-app.js # Runs as ES module
 ```
 
 If the nearest parent `package.json` lacks a `"type"` field, or contains
-`"type": "commonjs"`, extensionless and `.js` files are treated as CommonJS.
-If the volume root is reached and no `package.json` is found,
-Node.js defers to the default, a `package.json` with no `"type"`
-field. "Extensionless" refers to file paths which do not contain
-an extension as opposed to optionally dropping a file extension in a specifier.
+`"type": "commonjs"`, `.js` files are treated as CommonJS. If the volume root is
+reached and no `package.json` is found, Node.js defers to the default, a
+`package.json` with no `"type"` field.
 
-`import` statements of `.js` and extensionless files are treated as ES modules
-if the nearest parent `package.json` contains `"type": "module"`.
+`import` statements of `.js` files are treated as ES modules if the nearest
+parent `package.json` contains `"type": "module"`.
 
 ```js
 // my-app.js, part of the same example as above
@@ -106,14 +102,13 @@ as ES modules and `.cjs` files are always treated as CommonJS.
 
 ### Package Scope and File Extensions
 
-A folder containing a `package.json` file, and all subfolders below that
-folder down until the next folder containing another `package.json`, is
-considered a _package scope_. The `"type"` field defines how `.js` and
-extensionless files should be treated within a particular `package.json` file’s
-package scope. Every package in a project’s `node_modules` folder contains its
-own `package.json` file, so each project’s dependencies have their own package
-scopes. A `package.json` lacking a `"type"` field is treated as if it contained
-`"type": "commonjs"`.
+A folder containing a `package.json` file, and all subfolders below that folder
+down until the next folder containing another `package.json`, is considered a
+_package scope_. The `"type"` field defines how `.js` files should be treated
+within a particular `package.json` file’s package scope. Every package in a
+project’s `node_modules` folder contains its own `package.json` file, so each
+project’s dependencies have their own package scopes. A `package.json` lacking a
+`"type"` field is treated as if it contained `"type": "commonjs"`.
 
 The package scope applies not only to initial entry points (`node my-app.js`)
 but also to files referenced by `import` statements and `import()` expressions.
@@ -1042,8 +1037,7 @@ a URL should be interpreted. This can be one of the following:
 ```js
 /**
  * @param {string} url
- * @param {object} context
- * @param {string} context.parentURL
+ * @param {object} context (currently empty)
  * @param {function} defaultGetFormat
  * @returns {object} response
  * @returns {string} response.format
@@ -1367,15 +1361,13 @@ updates.
 In the following algorithms, all subroutine errors are propagated as errors
 of these top-level routines unless stated otherwise.
 
-_isMain_ is **true** when resolving the Node.js application entry point.
-
 _defaultEnv_ is the conditional environment name priority array,
 `["node", "import"]`.
 
 <details>
 <summary>Resolver algorithm specification</summary>
 
-**ESM_RESOLVE**(_specifier_, _parentURL_, _isMain_)
+**ESM_RESOLVE**(_specifier_, _parentURL_)
 
 > 1. Let _resolvedURL_ be **undefined**.
 > 1. If _specifier_ is a valid URL, then
@@ -1396,7 +1388,7 @@ _defaultEnv_ is the conditional environment name priority array,
 > 1. If the file at _resolvedURL_ does not exist, then
 >    1. Throw a _Module Not Found_ error.
 > 1. Set _resolvedURL_ to the real path of _resolvedURL_.
-> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_, _isMain_).
+> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_).
 > 1. Load _resolvedURL_ as module format, _format_.
 
 **PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_)
@@ -1549,7 +1541,7 @@ _defaultEnv_ is the conditional environment name priority array,
 >       1. Return _resolved_.
 > 1. Throw a _Module Not Found_ error.
 
-**ESM_FORMAT**(_url_, _isMain_)
+**ESM_FORMAT**(_url_)
 
 > 1. Assert: _url_ corresponds to an existing file.
 > 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_).
@@ -1558,12 +1550,10 @@ _defaultEnv_ is the conditional environment name priority array,
 > 1. If _url_ ends in _".cjs"_, then
 >    1. Return _"commonjs"_.
 > 1. If _pjson?.type_ exists and is _"module"_, then
->    1. If _isMain_ is **true** or _url_ ends in _".js"_, then
+>    1. If _url_ ends in _".js"_, then
 >       1. Return _"module"_.
 >    1. Throw an _Unsupported File Extension_ error.
 > 1. Otherwise,
->    1. If _isMain_ is **true**, then
->       1. Return _"commonjs"_.
 >    1. Throw an _Unsupported File Extension_ error.
 
 **READ_PACKAGE_SCOPE**(_url_)
diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -54,11 +54,13 @@ function defaultGetFormat(url, context, defaultGetFormat) {
     return { format };
   } else if (parsed.protocol === 'file:') {
     const ext = extname(parsed.pathname);
-    let format = extensionFormatMap[ext];
-    const isMain = context.parentURL === undefined;
-    if (ext === '.js' || (!format && isMain))
+    let format;
+    if (ext === '.js') {
       format = getPackageType(parsed.href) === TYPE_MODULE ?
         'module' : 'commonjs';
+    } else {
+      format = extensionFormatMap[ext];
+    }
     if (!format) {
       if (experimentalSpeciferResolution === 'node') {
         process.emitWarning(
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -96,7 +96,7 @@ class Loader {
     }
 
     const getFormatResponse = await this._getFormat(
-      url, { parentURL }, defaultGetFormat);
+      url, {}, defaultGetFormat);
     if (typeof getFormatResponse !== 'object') {
       throw new ERR_INVALID_RETURN_VALUE(
         'object', 'loader getFormat', getFormatResponse);
diff --git a/test/es-module/test-esm-no-extension.js b/test/es-module/test-esm-no-extension.js
diff --git a/test/es-module/test-esm-unknown-or-no-extension.js b/test/es-module/test-esm-unknown-or-no-extension.js
@@ -0,0 +1,36 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+const { spawn } = require('child_process');
+const assert = require('assert');
+
+// In a "type": "module" package scope, files with unknown extensions or no
+// extensions should throw; both when used as a main entry point and also when
+// referenced via `import`.
+
+[
+  '/es-modules/package-type-module/noext-esm',
+  '/es-modules/package-type-module/imports-noext.mjs',
+  '/es-modules/package-type-module/extension.unknown',
+  '/es-modules/package-type-module/imports-unknownext.mjs',
+].forEach((fixturePath) => {
+  const entry = fixtures.path(fixturePath);
+  const child = spawn(process.execPath, [entry]);
+  let stdout = '';
+  let stderr = '';
+  child.stderr.setEncoding('utf8');
+  child.stdout.setEncoding('utf8');
+  child.stdout.on('data', (data) => {
+    stdout += data;
+  });
+  child.stderr.on('data', (data) => {
+    stderr += data;
+  });
+  child.on('close', common.mustCall((code, signal) => {
+    assert.strictEqual(code, 1);
+    assert.strictEqual(signal, null);
+    assert.strictEqual(stdout, '');
+    assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
+  }));
+});
diff --git a/test/fixtures/es-modules/package-type-module/imports-noext.mjs b/test/fixtures/es-modules/package-type-module/imports-noext.mjs
@@ -0,0 +1 @@
+import './noext-esm';
diff --git a/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs b/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs
@@ -0,0 +1 @@
+import './extension.unknown';

Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ class Loader {`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`const getFormatResponse = await this._getFormat(`
`99`		`- url, { parentURL }, defaultGetFormat);`
	`99`	`+ url, {}, defaultGetFormat);`
`100`	`100`	`if (typeof getFormatResponse !== 'object') {`
`101`	`101`	`throw new ERR_INVALID_RETURN_VALUE(`
`102`	`102`	`'object', 'loader getFormat', getFormatResponse);`