esm: remove node specifier resolution algorithm

Refs: nodejs/modules#180

PR-URL: #6
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Author: MylesBorins

doc/api/esm.md

@@ -58,6 +58,14 @@ property:
 
 ESM must have the `.mjs` extension.
 
+### 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
@@ -82,21 +90,15 @@ 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.
 
-## Interop with existing modules
+## CommonJS,  JSON, and Native Modules
 
-CommonJS and C++ modules can be used with `import`.
-
-Modules loaded this way will only be loaded once, even if their query
-or fragment string differs between `import` statements.
-
-When loaded via `import` these modules will provide a single `default` export
-representing the value of `module.exports` at the time they finished evaluating.
+CommonJS, JSON, and Native modules can be used with [`module.createRequireFromPath()`][`module.createRequireFromPath()`].
 
 ```js
 // cjs.js
@@ -112,6 +114,8 @@ const cjs = require('./cjs');
 cjs === 'cjs'; // true
 ```
 
+## Builtin modules
+
 Builtin modules will provide named exports of their public API, as well as a
 default export which can be used for, among other things, modifying the named
 exports. Named exports of builtin modules are updated when the corresponding
@@ -261,3 +265,4 @@ in the import tree.
 
 [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md
 [dynamic instantiate hook]: #esm_dynamic_instantiate_hook
+[`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename

lib/internal/modules/cjs/loader.js

@@ -44,6 +44,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,
@@ -755,7 +756,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(pathToFileURL(process.argv[1]).pathname);

src/module_wrap.cc

@@ -26,7 +26,6 @@ using v8::HandleScope;
 using v8::Integer;
 using v8::IntegrityLevel;
 using v8::Isolate;
-using v8::JSON;
 using v8::Just;
 using v8::Local;
 using v8::Maybe;
@@ -505,70 +504,17 @@ Maybe<uv_file> CheckFile(const std::string& path,
   return Just(fd);
 }
 
-using Exists = PackageConfig::Exists;
-using IsValid = PackageConfig::IsValid;
-using HasMain = PackageConfig::HasMain;
-
-const PackageConfig& GetPackageConfig(Environment* env,
-                                      const std::string& path) {
-  auto existing = env->package_json_cache.find(path);
-  if (existing != env->package_json_cache.end()) {
-    return existing->second;
-  }
-  Maybe<uv_file> check = CheckFile(path, LEAVE_OPEN_AFTER_CHECK);
-  if (check.IsNothing()) {
-    auto entry = env->package_json_cache.emplace(path,
-        PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" });
-    return entry.first->second;
-  }
-
-  Isolate* isolate = env->isolate();
-  v8::HandleScope handle_scope(isolate);
-
-  std::string pkg_src = ReadFile(check.FromJust());
-  uv_fs_t fs_req;
-  CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr));
-  uv_fs_req_cleanup(&fs_req);
-
-  Local<String> src;
-  if (!String::NewFromUtf8(isolate,
-                           pkg_src.c_str(),
-                           v8::NewStringType::kNormal,
-                           pkg_src.length()).ToLocal(&src)) {
-    auto entry = env->package_json_cache.emplace(path,
-        PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" });
-    return entry.first->second;
-  }
-
-  Local<Value> pkg_json_v;
-  Local<Object> pkg_json;
-
-  if (!JSON::Parse(env->context(), src).ToLocal(&pkg_json_v) ||
-      !pkg_json_v->ToObject(env->context()).ToLocal(&pkg_json)) {
-    auto entry = env->package_json_cache.emplace(path,
-        PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "" });
-    return entry.first->second;
-  }
-
-  Local<Value> pkg_main;
-  HasMain has_main = HasMain::No;
-  std::string main_std;
-  if (pkg_json->Get(env->context(), env->main_string()).ToLocal(&pkg_main)) {
-    has_main = HasMain::Yes;
-    Utf8Value main_utf8(isolate, pkg_main);
-    main_std.assign(std::string(*main_utf8, main_utf8.length()));
-  }
-
-  auto entry = env->package_json_cache.emplace(path,
-      PackageConfig { Exists::Yes, IsValid::Yes, has_main, main_std });
-  return entry.first->second;
-}
-
 enum ResolveExtensionsOptions {
   TRY_EXACT_NAME,
   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) {
@@ -594,24 +540,6 @@ inline Maybe<URL> ResolveIndex(const URL& search) {
   return ResolveExtensions<ONLY_VIA_EXTENSIONS>(URL("index", search));
 }
 
-Maybe<URL> ResolveMain(Environment* env, const URL& search) {
-  URL pkg("package.json", &search);
-
-  const PackageConfig& pjson =
-      GetPackageConfig(env, pkg.ToFilePath());
-  // Note invalid package.json should throw in resolver
-  // currently we silently ignore which is incorrect
-  if (pjson.exists == Exists::No ||
-      pjson.is_valid == IsValid::No ||
-      pjson.has_main == HasMain::No) {
-    return Nothing<URL>();
-  }
-  if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) {
-    return Resolve(env, "./" + pjson.main, search, IgnoreMain);
-  }
-  return Resolve(env, pjson.main, search, IgnoreMain);
-}
-
 Maybe<URL> ResolveModule(Environment* env,
                          const std::string& specifier,
                          const URL& base) {
@@ -620,7 +548,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 =
@@ -640,23 +568,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
@@ -671,13 +587,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,26 +12,9 @@
 namespace node {
 namespace loader {
 
-enum PackageMainCheck : bool {
-    CheckMain = true,
-    IgnoreMain = false
-};
-
-enum ScriptType : int {
-  kScript,
-  kModule,
-};
-
-enum HostDefinedOptions : int {
-  kType = 8,
-  kID = 9,
-  kLength = 10,
-};
-
 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,5 +1,6 @@
 // Flags: --experimental-modules
-import '../common';
+/* eslint-disable node-core/required-modules */
+import '../common/index.mjs';
 
 // Assert we can import files with `%` in their pathname.
 

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);