module: add specific error for dir import

PR-URL: #33220 Fixes: #33219 Reviewed-By: Guy Bedford <[email protected]>
nodejs · May 16, 2020 · 35aae31 · 35aae31
1 parent 8f91338
commit 35aae31
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 10 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -2045,6 +2045,20 @@ An attempt was made to load a module with an unknown or unsupported format.
 An invalid or unknown process signal was passed to an API expecting a valid
 signal (such as [`subprocess.kill()`][]).
 
+<a id="ERR_UNSUPPORTED_DIR_IMPORT"></a>
+### `ERR_UNSUPPORTED_DIR_IMPORT`
+
+`import` a directory URL is unsupported. Instead, you can
+[self-reference a package using its name][] and [define a custom subpath][] in
+the `"exports"` field of the `package.json` file.
+
+<!-- eslint-skip -->
+```js
+import './'; // unsupported
+import './index.js'; // supported
+import 'package-name'; // supported
+```
+
 <a id="ERR_UNSUPPORTED_ESM_URL_SCHEME"></a>
 ### `ERR_UNSUPPORTED_ESM_URL_SCHEME`
 
@@ -2604,3 +2618,5 @@ such as `process.stdout.on('data')`.
 [Subresource Integrity specification]: https://www.w3.org/TR/SRI/#the-integrity-attribute
 [try-catch]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch
 [vm]: vm.html
+[self-reference a package using its name]: esm.html#esm_self_referencing_a_package_using_its_name
+[define a custom subpath]: esm.html#esm_subpath_exports
diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -1603,8 +1603,9 @@ The resolver can throw the following errors:
 > 1. If _resolvedURL_ contains any percent encodings of _"/"_ or _"\\"_ (_"%2f"_
 >    and _"%5C"_ respectively), then
 >    1. Throw an _Invalid Module Specifier_ error.
-> 1. If _resolvedURL_ does not end with a trailing _"/"_ and the file at
->    _resolvedURL_ does not exist, then
+> 1. If the file at _resolvedURL_ is a directory, then
+>    1. Throw an _Unsupported Directory Import_ error.
+> 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_).

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1411,6 +1411,8 @@ E('ERR_UNKNOWN_FILE_EXTENSION',
   TypeError);
 E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
 E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
+E('ERR_UNSUPPORTED_DIR_IMPORT', "Directory import '%s' is not supported " +
+'resolving ES modules, imported from %s', Error);
 E('ERR_UNSUPPORTED_ESM_URL_SCHEME', 'Only file and data URLs are supported ' +
   'by the default ESM loader', Error);
 

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -12,6 +12,7 @@ const {
   RegExp,
   SafeMap,
   SafeSet,
+  String,
   StringPrototypeEndsWith,
   StringPrototypeIncludes,
   StringPrototypeIndexOf,
@@ -48,6 +49,7 @@ const {
   ERR_INVALID_PACKAGE_TARGET,
   ERR_MODULE_NOT_FOUND,
   ERR_PACKAGE_PATH_NOT_EXPORTED,
+  ERR_UNSUPPORTED_DIR_IMPORT,
   ERR_UNSUPPORTED_ESM_URL_SCHEME,
 } = require('internal/errors').codes;
 
@@ -270,10 +272,15 @@ function finalizeResolution(resolved, base) {
       resolved.pathname, fileURLToPath(base), 'module');
   }
 
-  if (StringPrototypeEndsWith(resolved.pathname, '/')) return resolved;
   const path = fileURLToPath(resolved);
-
-  if (!tryStatSync(path).isFile()) {
+  const stats = tryStatSync(path);
+
+  if (stats.isDirectory()) {
+    const err = new ERR_UNSUPPORTED_DIR_IMPORT(
+      path || resolved.pathname, fileURLToPath(base));
+    err.url = String(resolved);
+    throw err;
+  } else if (!stats.isFile()) {
     throw new ERR_MODULE_NOT_FOUND(
       path || resolved.pathname, fileURLToPath(base), 'module');
   }
@@ -749,7 +756,8 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) {
   } catch (error) {
     // Try to give the user a hint of what would have been the
     // resolved CommonJS module
-    if (error.code === 'ERR_MODULE_NOT_FOUND') {
+    if (error.code === 'ERR_MODULE_NOT_FOUND' ||
+        error.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
       const found = resolveAsCommonJS(specifier, parentURL);
       if (found) {
         // Modify the stack and message string to include the hint

diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -5,6 +5,8 @@
 const {
   JSONParse,
   ObjectKeys,
+  PromisePrototypeCatch,
+  PromiseReject,
   SafeMap,
   StringPrototypeReplace,
 } = primordials;
@@ -58,7 +60,12 @@ function createImportMetaResolve(defaultParentUrl) {
     if (!esmLoader) {
       esmLoader = require('internal/process/esm_loader').ESMLoader;
     }
-    return esmLoader.resolve(specifier, parentUrl);
+    return PromisePrototypeCatch(
+      esmLoader.resolve(specifier, parentUrl),
+      (error) => (
+        error.code === 'ERR_UNSUPPORTED_DIR_IMPORT' ?
+          error.url : PromiseReject(error))
+    );
   };
 }
 

diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs
@@ -141,9 +141,13 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
   ]);
 
   if (!isRequire) {
+    const onDirectoryImport = (err) => {
+      strictEqual(err.code, 'ERR_UNSUPPORTED_DIR_IMPORT');
+      assertStartsWith(err.message, 'Directory import');
+    };
     notFoundExports.set('pkgexports/subpath/file', 'pkgexports/subpath/file');
-    notFoundExports.set('pkgexports/subpath/dir1', 'pkgexports/subpath/dir1');
-    notFoundExports.set('pkgexports/subpath/dir2', 'pkgexports/subpath/dir2');
+    loadFixture('pkgexports/subpath/dir1').catch(mustCall(onDirectoryImport));
+    loadFixture('pkgexports/subpath/dir2').catch(mustCall(onDirectoryImport));
   }
 
   for (const [specifier, request] of notFoundExports) {

diff --git a/test/es-module/test-esm-main-lookup.mjs b/test/es-module/test-esm-main-lookup.mjs
@@ -6,7 +6,7 @@ async function main() {
   try {
     mod = await import('../fixtures/es-modules/pjson-main');
   } catch (e) {
-    assert.strictEqual(e.code, 'ERR_MODULE_NOT_FOUND');
+    assert.strictEqual(e.code, 'ERR_UNSUPPORTED_DIR_IMPORT');
   }
 
   assert.strictEqual(mod, undefined);

diff --git a/test/parallel/test-directory-import.js b/test/parallel/test-directory-import.js
@@ -0,0 +1,14 @@
+'use strict';
+
+require('../common');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+const { pathToFileURL } = require('url');
+
+{
+  assert.rejects(import('./'), /ERR_UNSUPPORTED_DIR_IMPORT/);
+  assert.rejects(
+    import(pathToFileURL(fixtures.path('packages', 'main'))),
+    /Did you mean/,
+  );
+}