From 11cb078552711a6fb7b7154f41799697a4f6d790 Mon Sep 17 00:00:00 2001 From: Antoine du HAMEL Date: Sun, 3 May 2020 12:04:59 +0200 Subject: [PATCH 1/7] module: add specific error for dir import Fixes: https://github.com/nodejs/node/issues/33219 --- doc/api/errors.md | 14 ++++++++++++++ doc/api/esm.md | 5 +++-- lib/internal/errors.js | 1 + lib/internal/modules/esm/resolve.js | 16 ++++++++++++---- lib/internal/modules/esm/translators.js | 7 ++++++- test/es-module/test-esm-exports.mjs | 2 -- test/es-module/test-esm-main-lookup.mjs | 2 +- test/parallel/test-directory-import.js | 10 ++++++++++ 8 files changed, 47 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 172eadf7686794..8b173ecc7cf962 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2031,6 +2031,19 @@ 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][]. + + +```js +import './'; // unsupported +import './index.js'; // supported +import 'package-name'; // supported +``` + ### `ERR_UNSUPPORTED_ESM_URL_SCHEME` @@ -2585,3 +2598,4 @@ 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 diff --git a/doc/api/esm.md b/doc/api/esm.md index 5047600f988f66..ac14da9291e5da 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1506,8 +1506,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 a _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 2a162f14b24206..c37d176a333994 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1403,6 +1403,7 @@ 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', "Cannot import '%s' 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..f56a2714863ca4 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -5,6 +5,7 @@ const { JSONParse, ObjectKeys, + PromiseReject, SafeMap, StringPrototypeReplace, } = primordials; @@ -58,7 +59,11 @@ function createImportMetaResolve(defaultParentUrl) { if (!esmLoader) { esmLoader = require('internal/process/esm_loader').ESMLoader; } - return esmLoader.resolve(specifier, parentUrl); + return esmLoader.resolve(specifier, parentUrl) + .catch((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..4153b34523eaef 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -142,8 +142,6 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; if (!isRequire) { notFoundExports.set('pkgexports/subpath/file', 'pkgexports/subpath/file'); - notFoundExports.set('pkgexports/subpath/dir1', 'pkgexports/subpath/dir1'); - notFoundExports.set('pkgexports/subpath/dir2', 'pkgexports/subpath/dir2'); } 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..d51d6b3155afab --- /dev/null +++ b/test/parallel/test-directory-import.js @@ -0,0 +1,10 @@ +'use strict'; + +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); + +{ + assert.rejects(import('./'), /ERR_UNSUPPORTED_DIR_IMPORT/); + assert.rejects(import(fixtures.path('packages', 'main')), /Did you mean/); +} From cb7fe74b7624338642cea573c551475ccdb136ea Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 5 May 2020 00:48:45 +0200 Subject: [PATCH 2/7] primodials Co-authored-by: Jordan Harband --- lib/internal/modules/esm/translators.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index f56a2714863ca4..a69fe10e0d4d6f 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -59,8 +59,9 @@ function createImportMetaResolve(defaultParentUrl) { if (!esmLoader) { esmLoader = require('internal/process/esm_loader').ESMLoader; } - return esmLoader.resolve(specifier, parentUrl) - .catch((error) => ( + return PromisePrototypeCatch( + esmLoader.resolve(specifier, parentUrl), + (error) => ( error.code === 'ERR_UNSUPPORTED_DIR_IMPORT' ? error.url : PromiseReject(error)) ); From 30cd02e34d6811204ecc6957e1fea04743cedadb Mon Sep 17 00:00:00 2001 From: Antoine du HAMEL Date: Tue, 5 May 2020 00:50:04 +0200 Subject: [PATCH 3/7] fixup: primordials --- lib/internal/modules/esm/translators.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index a69fe10e0d4d6f..e04dd80cae8759 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -5,6 +5,7 @@ const { JSONParse, ObjectKeys, + PromisePrototypeCatch, PromiseReject, SafeMap, StringPrototypeReplace, From e3da38df560c9c89299211ccb74681da6c22ac31 Mon Sep 17 00:00:00 2001 From: Antoine du HAMEL Date: Tue, 5 May 2020 12:13:00 +0200 Subject: [PATCH 4/7] fixup --- doc/api/errors.md | 4 +++- lib/internal/errors.js | 3 ++- lib/internal/modules/esm/translators.js | 2 +- test/es-module/test-esm-exports.mjs | 6 ++++++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 8b173ecc7cf962..5ad281e36c0dad 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2035,7 +2035,8 @@ 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][]. +[self-reference a package using its name][] and [define a custom subpath][] in +the `"exports"` field of the `package.json` file. ```js @@ -2599,3 +2600,4 @@ such as `process.stdout.on('data')`. [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/lib/internal/errors.js b/lib/internal/errors.js index c37d176a333994..f9c97238b0b810 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1403,7 +1403,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', "Cannot import '%s' from %s.", Error); +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/translators.js b/lib/internal/modules/esm/translators.js index e04dd80cae8759..a7252c1c99f954 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -65,7 +65,7 @@ function createImportMetaResolve(defaultParentUrl) { (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 4153b34523eaef..f9dc6ec472f424 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -141,7 +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'); + loadFixture('pkgexports/subpath/dir1').catch(mustCall(onDirectoryImport)); + loadFixture('pkgexports/subpath/dir2').catch(mustCall(onDirectoryImport)); } for (const [specifier, request] of notFoundExports) { From 1471091bd65c3b7ea4e3c5ac445e1ff5d8b4ca44 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 5 May 2020 17:05:39 +0200 Subject: [PATCH 5/7] fixup: nit Co-authored-by: Jordan Harband --- doc/api/esm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index ac14da9291e5da..540c05014a335d 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1507,7 +1507,7 @@ The resolver can throw the following errors: > and _"%5C"_ respectively), then > 1. Throw an _Invalid Module Specifier_ error. > 1. If the file at _resolvedURL_ is a directory, then -> 1. Throw a _Unsupported Directory Import_ error. +> 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_. From 112c5525693f482ed819d4dd7b1e2ae4a662d8a3 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 14 May 2020 20:44:23 +0200 Subject: [PATCH 6/7] fixup: Windows tests --- test/parallel/test-directory-import.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-directory-import.js b/test/parallel/test-directory-import.js index d51d6b3155afab..83fd01f6a0f214 100644 --- a/test/parallel/test-directory-import.js +++ b/test/parallel/test-directory-import.js @@ -3,8 +3,12 @@ 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(fixtures.path('packages', 'main')), /Did you mean/); + assert.rejects( + import(pathToFileURL(fixtures.path('packages', 'main'))), + /Did you mean/, + ); } From ed3dfc5ee925c94dd832da3aa56479528ea4db3d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 15 May 2020 01:24:24 +0200 Subject: [PATCH 7/7] esm: improve commonjs hint on module not found Adds hint when module specifier is a file URL. Refs: https://github.com/nodejs/node/pull/31906 --- lib/internal/modules/esm/resolve.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 8bff3fc9dfd251..cbcdb020ed7cd3 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -758,6 +758,9 @@ function defaultResolve(specifier, context = {}, defaultResolveUnused) { // resolved CommonJS module if (error.code === 'ERR_MODULE_NOT_FOUND' || error.code === 'ERR_UNSUPPORTED_DIR_IMPORT') { + if (StringPrototypeStartsWith(specifier, 'file://')) { + specifier = fileURLToPath(specifier); + } const found = resolveAsCommonJS(specifier, parentURL); if (found) { // Modify the stack and message string to include the hint