From 35aae319689a073838b845ea50d7c45631ddfc39 Mon Sep 17 00:00:00 2001 From: Antoine du HAMEL Date: Sun, 3 May 2020 12:04:59 +0200 Subject: [PATCH] module: add specific error for dir import PR-URL: https://github.com/nodejs/node/pull/33220 Fixes: https://github.com/nodejs/node/issues/33219 Reviewed-By: Guy Bedford --- doc/api/errors.md | 16 ++++++++++++++++ doc/api/esm.md | 5 +++-- lib/internal/errors.js | 2 ++ lib/internal/modules/esm/resolve.js | 16 ++++++++++++---- lib/internal/modules/esm/translators.js | 9 ++++++++- test/es-module/test-esm-exports.mjs | 8 ++++++-- test/es-module/test-esm-main-lookup.mjs | 2 +- test/parallel/test-directory-import.js | 14 ++++++++++++++ 8 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-directory-import.js diff --git a/doc/api/errors.md b/doc/api/errors.md index a17460993431ec..e25b6d9d5c7d61 100644 --- 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()`][]). + +### `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. + + +```js +import './'; // unsupported +import './index.js'; // supported +import 'package-name'; // supported +``` + ### `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 index f3a34e7d463e41..365f19da582f5f 100644 --- 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 index 0d5e7452a39db1..61fdf2ff7b4a5f 100644 --- 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 index c66e0554ad2746..8bff3fc9dfd251 100644 --- 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 index 497f90ed94475b..a7252c1c99f954 100644 --- 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 index 74bf66be32c39a..f9dc6ec472f424 100644 --- 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 index 2023a105e4dc42..4694d1c8e626e0 100644 --- 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 new file mode 100644 index 00000000000000..83fd01f6a0f214 --- /dev/null +++ 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/, + ); +}