Skip to content

Commit

Permalink
module: add specific error for dir import
Browse files Browse the repository at this point in the history
PR-URL: #33220
Fixes: #33219
Reviewed-By: Guy Bedford <guybedford@gmail.com>
  • Loading branch information
aduh95 authored and codebytere committed Jun 9, 2020
1 parent a0bc2e3 commit 4f6e4ae
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 10 deletions.
16 changes: 16 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2023,6 +2023,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`

Expand Down Expand Up @@ -2563,3 +2577,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
5 changes: 3 additions & 2 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1581,8 +1581,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_).
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,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);

Expand Down
16 changes: 12 additions & 4 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
RegExp,
SafeMap,
SafeSet,
String,
StringPrototypeEndsWith,
StringPrototypeIncludes,
StringPrototypeIndexOf,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
const {
JSONParse,
ObjectKeys,
PromisePrototypeCatch,
PromiseReject,
SafeMap,
StringPrototypeReplace,
} = primordials;
Expand Down Expand Up @@ -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))
);
};
}

Expand Down
8 changes: 6 additions & 2 deletions test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,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) {
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-main-lookup.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-directory-import.js
Original file line number Diff line number Diff line change
@@ -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/,
);
}

0 comments on commit 4f6e4ae

Please sign in to comment.