modules: remove file extension and directory resolving for esm

doc/api/esm.md

@@ -54,6 +54,14 @@ property:
 
 ## Notable differences between `import` and `require`
 
+### Mandatory file extensions
+
+You must provide a file extension when using the `import` keyword.
+
+### No importing directories
+
+There is no support for importing directories.
+
 ### No NODE_PATH
 
 `NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks
@@ -78,8 +86,8 @@ Modules will be loaded multiple times if the `import` specifier used to resolve
 them have a different query or fragment.
 
 ```js
-import './foo?query=1'; // loads ./foo with query of "?query=1"
-import './foo?query=2'; // loads ./foo with query of "?query=2"
+import './foo.mjs?query=1'; // loads ./foo.mjs with query of "?query=1"
+import './foo.mjs?query=2'; // loads ./foo.mjs with query of "?query=2"
 ```
 
 For now, only modules using the `file:` protocol can be loaded.

lib/internal/modules/cjs/loader.js

@@ -42,6 +42,7 @@ const {
 const preserveSymlinks = !!process.binding('config').preserveSymlinks;
 const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain;
 const experimentalModules = !!process.binding('config').experimentalModules;
+const hasLoader = !!process.binding('config').userLoader;
 
 const {
   ERR_INVALID_ARG_TYPE,
@@ -728,7 +729,11 @@ if (experimentalModules) {
 // bootstrap main module.
 Module.runMain = function() {
   // Load the main module--the command line argument.
-  if (experimentalModules) {
+  const base = path.basename(process.argv[1]);
+  const ext = path.extname(base);
+  const isESM = ext === '.mjs';
+
+  if (experimentalModules && (isESM || hasLoader)) {
     if (asyncESM === undefined) lazyLoadESM();
     asyncESM.loaderPromise.then((loader) => {
       return loader.import(getURLFromFilePath(process.argv[1]).pathname);

src/module_wrap.cc

@@ -552,6 +552,12 @@ enum ResolveExtensionsOptions {
   ONLY_VIA_EXTENSIONS
 };
 
+inline bool ResolvesToFile(const URL& search) {
+  std::string filePath = search.ToFilePath();
+  Maybe<uv_file> check = CheckFile(filePath);
+  return !check.IsNothing();
+}
+
 template <ResolveExtensionsOptions options>
 Maybe<URL> ResolveExtensions(const URL& search) {
   if (options == TRY_EXACT_NAME) {
@@ -589,10 +595,20 @@ Maybe<URL> ResolveMain(Environment* env, const URL& search) {
       pjson.has_main == HasMain::No) {
     return Nothing<URL>();
   }
-  if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) {
-    return Resolve(env, "./" + pjson.main, search, IgnoreMain);
+
+  URL resolved;
+  if (ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) {
+    resolved = URL(pjson.main, search);
+  } else {
+    resolved = URL("./" + pjson.main, search);
+  }
+  Maybe<URL> file = ResolveExtensions<TRY_EXACT_NAME>(resolved);
+  if (!file.IsNothing())
+    return file;
+  if (pjson.main.back() != '/') {
+    resolved = URL(pjson.main + "/", search);
   }
-  return Resolve(env, pjson.main, search, IgnoreMain);
+  return ResolveIndex(resolved);
 }
 
 Maybe<URL> ResolveModule(Environment* env,
@@ -603,7 +619,7 @@ Maybe<URL> ResolveModule(Environment* env,
   do {
     dir = parent;
     Maybe<URL> check =
-        Resolve(env, "./node_modules/" + specifier, dir, CheckMain);
+        Resolve(env, "./node_modules/" + specifier, dir);
     if (!check.IsNothing()) {
       const size_t limit = specifier.find('/');
       const size_t spec_len =
@@ -623,23 +639,11 @@ Maybe<URL> ResolveModule(Environment* env,
   return Nothing<URL>();
 }
 
-Maybe<URL> ResolveDirectory(Environment* env,
-                            const URL& search,
-                            PackageMainCheck check_pjson_main) {
-  if (check_pjson_main) {
-    Maybe<URL> main = ResolveMain(env, search);
-    if (!main.IsNothing())
-      return main;
-  }
-  return ResolveIndex(search);
-}
-
 }  // anonymous namespace
 
 Maybe<URL> Resolve(Environment* env,
                    const std::string& specifier,
-                   const URL& base,
-                   PackageMainCheck check_pjson_main) {
+                   const URL& base) {
   URL pure_url(specifier);
   if (!(pure_url.flags() & URL_FLAGS_FAILED)) {
     // just check existence, without altering
@@ -654,13 +658,9 @@ Maybe<URL> Resolve(Environment* env,
   }
   if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
     URL resolved(specifier, base);
-    Maybe<URL> file = ResolveExtensions<TRY_EXACT_NAME>(resolved);
-    if (!file.IsNothing())
-      return file;
-    if (specifier.back() != '/') {
-      resolved = URL(specifier + "/", base);
-    }
-    return ResolveDirectory(env, resolved, check_pjson_main);
+    if (ResolvesToFile(resolved))
+      return Just(resolved);
+    return Nothing<URL>();
   } else {
     return ResolveModule(env, specifier, base);
   }

src/module_wrap.h

@@ -12,15 +12,9 @@
 namespace node {
 namespace loader {
 
-enum PackageMainCheck : bool {
-    CheckMain = true,
-    IgnoreMain = false
-};
-
 v8::Maybe<url::URL> Resolve(Environment* env,
                             const std::string& specifier,
-                            const url::URL& base,
-                            PackageMainCheck read_pkg_json = CheckMain);
+                            const url::URL& base);
 
 class ModuleWrap : public BaseObject {
  public:

test/es-module/test-esm-basic-imports.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 import assert from 'assert';
 import ok from '../fixtures/es-modules/test-esm-ok.mjs';
 import okShebang from './test-esm-shebang.mjs';

test/es-module/test-esm-cyclic-dynamic-import.mjs

@@ -1,3 +1,4 @@
 // Flags: --experimental-modules
-import '../common';
-import('./test-esm-cyclic-dynamic-import');
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
+import('./test-esm-cyclic-dynamic-import.mjs');

test/es-module/test-esm-double-encoding.mjs

@@ -1,6 +1,7 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 
 // Assert we can import files with `%` in their pathname.
 
-import '../fixtures/es-modules/test-esm-double-encoding-native%2520.js';
+import '../fixtures/es-modules/test-esm-double-encoding-native%2520.mjs';

test/es-module/test-esm-encoded-path.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 import assert from 'assert';
 // ./test-esm-ok.mjs
 import ok from '../fixtures/es-modules/test-%65%73%6d-ok.mjs';

test/es-module/test-esm-forbidden-globals.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 
 // eslint-disable-next-line no-undef
 if (typeof arguments !== 'undefined') {

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

@@ -1,6 +1,7 @@
 // Flags: --experimental-modules
+/* eslint-disable node-core/required-modules */
 
-import '../common';
+import '../common/index.mjs';
 import assert from 'assert';
 
 assert.strictEqual(Object.getPrototypeOf(import.meta), null);

test/es-module/test-esm-json.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 import assert from 'assert';
 import ok from '../fixtures/es-modules/test-esm-ok.mjs';
 import json from '../fixtures/es-modules/json.json';

test/es-module/test-esm-live-binding.mjs

@@ -1,6 +1,7 @@
 // Flags: --experimental-modules
+/* eslint-disable node-core/required-modules */
 
-import '../common';
+import '../common/index.mjs';
 import assert from 'assert';
 
 import fs, { readFile, readFileSync } from 'fs';

test/es-module/test-esm-loader-invalid-format.mjs

@@ -1,5 +1,5 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs
-import { expectsError, mustCall } from '../common';
+import { expectsError, mustCall } from '../common/index.mjs';
 import assert from 'assert';
 
 import('../fixtures/es-modules/test-esm-ok.mjs')

test/es-module/test-esm-loader-invalid-url.mjs

@@ -1,5 +1,5 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs
-import { expectsError, mustCall } from '../common';
+import { expectsError, mustCall } from '../common/index.mjs';
 import assert from 'assert';
 
 import('../fixtures/es-modules/test-esm-ok.mjs')

test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs

@@ -1,6 +1,6 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs
 
-import { expectsError } from '../common';
+import { expectsError } from '../common/index.mjs';
 
 import('test').catch(expectsError({
   code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK',

test/es-module/test-esm-named-exports.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 import { readFile } from 'fs';
 import assert from 'assert';
 import ok from '../fixtures/es-modules/test-esm-ok.mjs';

test/es-module/test-esm-namespace.mjs

@@ -1,5 +1,7 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+
+import '../common/index.mjs';
 import * as fs from 'fs';
 import assert from 'assert';
 import Module from 'module';

test/es-module/test-esm-preserve-symlinks-not-found.mjs

@@ -1,3 +1,3 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs
 /* eslint-disable node-core/required-modules */
-import './not-found';
+import './not-found.mjs';

test/es-module/test-esm-require-cache.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 import '../fixtures/es-module-require-cache/preload.js';
 import '../fixtures/es-module-require-cache/counter.js';
 import assert from 'assert';

test/es-module/test-esm-shared-loader-dep.mjs

@@ -1,5 +1,6 @@
 // Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 import assert from 'assert';
 import '../fixtures/es-modules/test-esm-ok.mjs';
 import dep from '../fixtures/es-module-loaders/loader-dep.js';

test/es-module/test-esm-shebang.mjs

@@ -1,6 +1,7 @@
 #! }]) // isn't js
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 
 const isJs = true;
 export default isJs;

test/es-module/test-esm-snapshot.mjs

@@ -1,7 +1,8 @@
 // Flags: --experimental-modules
-import '../common';
-import '../fixtures/es-modules/esm-snapshot-mutator';
-import one from '../fixtures/es-modules/esm-snapshot';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
+import '../fixtures/es-modules/esm-snapshot-mutator.js';
+import one from '../fixtures/es-modules/esm-snapshot.js';
 import assert from 'assert';
 
 assert.strictEqual(one, 1);

test/es-module/test-esm-symlink-main.js

@@ -9,7 +9,7 @@ const fs = require('fs');
 tmpdir.refresh();
 
 const realPath = path.resolve(__dirname, '../fixtures/es-modules/symlink.mjs');
-const symlinkPath = path.resolve(tmpdir.path, 'symlink.js');
+const symlinkPath = path.resolve(tmpdir.path, 'symlink.mjs');
 
 try {
   fs.symlinkSync(realPath, symlinkPath);

test/es-module/test-esm-symlink.js

@@ -12,8 +12,8 @@ const tmpDir = tmpdir.path;
 
 const entry = path.join(tmpDir, 'entry.mjs');
 const real = path.join(tmpDir, 'index.mjs');
-const link_absolute_path = path.join(tmpDir, 'absolute');
-const link_relative_path = path.join(tmpDir, 'relative');
+const link_absolute_path = path.join(tmpDir, 'absolute.mjs');
+const link_relative_path = path.join(tmpDir, 'relative.mjs');
 const link_ignore_extension = path.join(tmpDir,
                                         'ignore_extension.json');
 const link_directory = path.join(tmpDir, 'directory');
@@ -22,15 +22,13 @@ fs.writeFileSync(real, 'export default [];');
 fs.writeFileSync(entry, `
 import assert from 'assert';
 import real from './index.mjs';
-import absolute from './absolute';
-import relative from './relative';
+import absolute from './absolute.mjs';
+import relative from './relative.mjs';
 import ignoreExtension from './ignore_extension.json';
-import directory from './directory';
 
 assert.strictEqual(absolute, real);
 assert.strictEqual(relative, real);
 assert.strictEqual(ignoreExtension, real);
-assert.strictEqual(directory, real);
 `);
 
 try {

test/es-module/test-esm-throw-undefined.mjs

@@ -1,11 +1,11 @@
 // Flags: --experimental-modules
-import '../common';
+import '../common/index.mjs';
 import assert from 'assert';
 
 async function doTest() {
   await assert.rejects(
     async () => {
-      await import('../fixtures/es-module-loaders/throw-undefined');
+      await import('../fixtures/es-module-loaders/throw-undefined.mjs');
     },
     (e) => e === undefined
   );

test/fixtures/es-module-loaders/syntax-error-import.mjs

@@ -1 +1 @@
-import { foo, notfound } from './module-named-exports';
+import { foo, notfound } from './module-named-exports.mjs';

test/fixtures/es-modules/loop.mjs

@@ -1,4 +1,4 @@
-import { message } from './message';
+import { message } from './message.mjs';
 
 var t = 1;
 var k = 1;

test/fixtures/es-modules/test-esm-double-encoding-native%20.mjs

@@ -0,0 +1,6 @@
+'use strict';
+
+// Trivial test to assert we can load files with `%` in their pathname.
+// Imported by `test-esm-double-encoding.mjs`.
+
+export default 42;

test/message/esm_display_syntax_error_import.mjs

@@ -1,7 +1,7 @@
 // Flags:  --experimental-modules
-/* eslint-disable no-unused-vars */
-import '../common';
+/* eslint-disable no-unused-vars, node-core/required-modules */
+import '../common/index.mjs';
 import {
   foo,
   notfound
-} from '../fixtures/es-module-loaders/module-named-exports';
+} from '../fixtures/es-module-loaders/module-named-exports.mjs';

test/message/esm_display_syntax_error_import.out

@@ -2,5 +2,5 @@
 file:///*/test/message/esm_display_syntax_error_import.mjs:6
   notfound
   ^^^^^^^^
-SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports' does not provide an export named 'notfound'
+SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports.mjs' does not provide an export named 'notfound'
     at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*)