Skip to content
This repository was archived by the owner on Apr 16, 2020. It is now read-only.

esm resolver spec and implementation refinements #12

Closed
wants to merge 3 commits into from
Closed

esm resolver spec and implementation refinements #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a0a78f
esm: minimal resolver spec and implementation refinements
guybedford Oct 24, 2018
cc73c36
newline lint fix
guybedford Nov 21, 2018
2c72b19
explicit string concatenation
guybedford Nov 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions doc/api/errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1393,26 +1393,19 @@ a `dynamicInstantiate` hook.
A `MessagePort` was found in the object passed to a `postMessage()` call,
but not provided in the `transferList` for that call.

<a id="ERR_MISSING_MODULE"></a>
### ERR_MISSING_MODULE

> Stability: 1 - Experimental

An [ES6 module][] could not be resolved.

<a id="ERR_MISSING_PLATFORM_FOR_WORKER"></a>
### ERR_MISSING_PLATFORM_FOR_WORKER

The V8 platform used by this instance of Node.js does not support creating
Workers. This is caused by lack of embedder support for Workers. In particular,
this error will not occur with standard builds of Node.js.

<a id="ERR_MODULE_RESOLUTION_LEGACY"></a>
### ERR_MODULE_RESOLUTION_LEGACY
<a id="ERR_MODULE_NOT_FOUND"></a>
### ERR_MODULE_NOT_FOUND

> Stability: 1 - Experimental

A failure occurred resolving imports in an [ES6 module][].
An [ESM module][] could not be resolved.

<a id="ERR_MULTIPLE_CALLBACK"></a>
### ERR_MULTIPLE_CALLBACK
Expand Down
50 changes: 46 additions & 4 deletions doc/api/esm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ ESM must have the `.mjs` extension.

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
Expand Down Expand Up @@ -146,6 +142,52 @@ fs.readFileSync = () => Buffer.from('Hello, ESM');
fs.readFileSync === readFileSync;
```

## Resolution Algorithm

### Features

The resolver has the following properties:

* FileURL-based resolution as is used by ES modules
* Support for builtin module loading
* Relative and absolute URL resolution
* No default extensions
* No folder mains
* Bare specifier package resolution lookup through node_modules
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should take on the historical burden of node_modules in the default implementation. It's inefficient, hard to predict (e.g. it might load random things from a user's home directory), and enshrines a directory layout that many people (myself included) are trying to get away from.

Should we maybe split this into scoped resolution and an example for how the underlying data for scoped resolution might be calculated from a directory tree?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, how is it hard to predict?

That directory layout isn't something that can be trivially nor any time soon gotten away from.


### Resolver Algorithm

The algorithm to resolve an ES module specifier is provided through
_ESM_RESOLVE_:

**ESM_RESOLVE**(_specifier_, _parentURL_)
> 1. Let _resolvedURL_ be _undefined_.
> 1. If _specifier_ is a valid URL then,
> 1. Set _resolvedURL_ to the result of parsing and reserializing
> _specifier_ as a URL.
> 1. Otherwise, if _specifier_ starts with _"/"_, _"./"_ or _"../"_ then,
> 1. Set _resolvedURL_ to the URL resolution of _specifier_ relative to
> _parentURL_.
> 1. Otherwise,
> 1. Note: _specifier_ is now a bare specifier.
> 1. Set _resolvedURL_ the result of
> **PACKAGE_RESOLVE**(_specifier_, _parentURL_).
> 1. If the file at _resolvedURL_ does not exist then,
> 1. Throw a _Module Not Found_ error.
> 1. Return _resolvedURL_.

**PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_)
> 1. Assert: _packageSpecifier_ is a bare specifier.
> 1. If _packageSpecifier_ is a Node.js builtin module then,
> 1. Return the string _"node:"_ concatenated with _packageSpecifier_.
> 1. While _parentURL_ contains a non-empty _pathname_,
> 1. Set _parentURL_ to the parent folder URL of _parentURL_.
> 1. Let _packageURL_ be the URL resolution of the string concatenation of
> _parentURL_, _"/node_modules/"_ and _"packageSpecifier"_.
> 1. If the file at _packageURL_ exists then,
> 1. Return _packageURL_.
> 1. Throw a _Module Not Found_ error.

[Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md
[`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename
[ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md
16 changes: 10 additions & 6 deletions lib/internal/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,11 +800,13 @@ E('ERR_MISSING_ARGS',
E('ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK',
'The ES Module loader may not return a format of \'dynamic\' when no ' +
'dynamicInstantiate function was provided', Error);
E('ERR_MISSING_MODULE', 'Cannot find module %s', Error);
E('ERR_MODULE_RESOLUTION_LEGACY',
'%s not found by import in %s.' +
' Legacy behavior in require() would have found it at %s',
Error);
E('ERR_MODULE_NOT_FOUND', (module, base, legacyResolution) => {
let msg = `Cannot find module '${module}' imported from ${base}.`;
if (legacyResolution)
msg += ' Legacy behavior in require() would have found it at ' +
legacyResolution;
return msg;
}, Error);
E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times', Error);
E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function', TypeError);
E('ERR_NAPI_INVALID_DATAVIEW_ARGS',
Expand Down Expand Up @@ -894,11 +896,13 @@ E('ERR_UNHANDLED_ERROR',
if (err === undefined) return msg;
return `${msg} (${err})`;
}, Error);
// This should probably be a `TypeError`.
E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);

// This should probably be a `TypeError`.
E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s', Error);
E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: \'%s\' imported ' +
'from %s', Error);
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type', Error);
Expand Down
27 changes: 6 additions & 21 deletions lib/internal/modules/esm/default_resolve.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,15 @@ const { getOptionValue } = require('internal/options');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const {
ERR_MISSING_MODULE,
ERR_MODULE_RESOLUTION_LEGACY,
ERR_MODULE_NOT_FOUND,
ERR_UNKNOWN_FILE_EXTENSION
} = require('internal/errors').codes;
const { resolve: moduleWrapResolve } = internalBinding('module_wrap');
const StringStartsWith = Function.call.bind(String.prototype.startsWith);
const { pathToFileURL, fileURLToPath } = require('internal/url');

const realpathCache = new Map();

function search(target, base) {
if (base === undefined) {
// We cannot search without a base.
throw new ERR_MISSING_MODULE(target);
}
try {
return moduleWrapResolve(target, base);
} catch (e) {
Expand All @@ -36,7 +30,7 @@ function search(target, base) {
tmpMod.paths = CJSmodule._nodeModulePaths(
new URL('./', questionedBase).pathname);
const found = CJSmodule._resolveFilename(target, tmpMod);
error = new ERR_MODULE_RESOLUTION_LEGACY(target, base, found);
error = new ERR_MODULE_NOT_FOUND(target, fileURLToPath(base), found);
} catch {
// ignore
}
Expand All @@ -57,16 +51,8 @@ function resolve(specifier, parentURL) {
};
}

let url;
try {
url = search(specifier,
parentURL || pathToFileURL(`${process.cwd()}/`).href);
} catch (e) {
if (typeof e.message === 'string' &&
StringStartsWith(e.message, 'Cannot find module'))
e.code = 'MODULE_NOT_FOUND';
throw e;
}
let url = search(specifier,
parentURL || pathToFileURL(`${process.cwd()}/`).href);

const isMain = parentURL === undefined;

Expand All @@ -87,12 +73,11 @@ function resolve(specifier, parentURL) {
if (isMain)
format = 'cjs';
else
throw new ERR_UNKNOWN_FILE_EXTENSION(url.pathname);
throw new ERR_UNKNOWN_FILE_EXTENSION(url.pathname,
fileURLToPath(parentURL));
}

return { url: `${url}`, format };
}

module.exports = resolve;
// exported for tests
module.exports.search = search;
158 changes: 50 additions & 108 deletions src/module_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,104 +466,37 @@ std::string ReadFile(uv_file file) {
return contents;
}

enum CheckFileOptions {
LEAVE_OPEN_AFTER_CHECK,
CLOSE_AFTER_CHECK
enum DescriptorType {
NONE,
FILE,
DIRECTORY
};

Maybe<uv_file> CheckFile(const std::string& path,
CheckFileOptions opt = CLOSE_AFTER_CHECK) {
DescriptorType CheckDescriptor(const std::string& path) {
uv_fs_t fs_req;
if (path.empty()) {
return Nothing<uv_file>();
}

uv_file fd = uv_fs_open(nullptr, &fs_req, path.c_str(), O_RDONLY, 0, nullptr);
uv_fs_req_cleanup(&fs_req);

if (fd < 0) {
return Nothing<uv_file>();
}

uv_fs_fstat(nullptr, &fs_req, fd, nullptr);
uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR;
uv_fs_req_cleanup(&fs_req);

if (is_directory) {
CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr));
uv_fs_req_cleanup(&fs_req);
return Nothing<uv_file>();
}

if (opt == CLOSE_AFTER_CHECK) {
CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr));
int rc = uv_fs_stat(nullptr, &fs_req, path.c_str(), nullptr);
if (rc == 0) {
uint64_t is_directory = fs_req.statbuf.st_mode & S_IFDIR;
uv_fs_req_cleanup(&fs_req);
return is_directory ? DIRECTORY : FILE;
}

return Just(fd);
}

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) {
std::string filePath = search.ToFilePath();
Maybe<uv_file> check = CheckFile(filePath);
if (!check.IsNothing()) {
return Just(search);
}
}

for (const char* extension : EXTENSIONS) {
URL guess(search.path() + extension, &search);
Maybe<uv_file> check = CheckFile(guess.ToFilePath());
if (!check.IsNothing()) {
return Just(guess);
}
}

return Nothing<URL>();
}

inline Maybe<URL> ResolveIndex(const URL& search) {
return ResolveExtensions<ONLY_VIA_EXTENSIONS>(URL("index", search));
uv_fs_req_cleanup(&fs_req);
return NONE;
}

Maybe<URL> ResolveModule(Environment* env,
const std::string& specifier,
const URL& base) {
Maybe<URL> PackageResolve(Environment* env,
const std::string& specifier,
const URL& base) {
URL parent(".", base);
URL dir("");
std::string last_path;
do {
dir = parent;
Maybe<URL> check =
Resolve(env, "./node_modules/" + specifier, dir);
if (!check.IsNothing()) {
const size_t limit = specifier.find('/');
const size_t spec_len =
limit == std::string::npos ? specifier.length() :
limit + 1;
std::string chroot =
dir.path() + "node_modules/" + specifier.substr(0, spec_len);
if (check.FromJust().path().substr(0, chroot.length()) != chroot) {
return Nothing<URL>();
}
return check;
} else {
// TODO(bmeck) PREVENT FALLTHROUGH
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this TODO is fixed :)

}
parent = URL("..", &dir);
} while (parent.path() != dir.path());
URL pkg_url("./node_modules/" + specifier, &parent);
DescriptorType check = CheckDescriptor(pkg_url.ToFilePath());
if (check == FILE) return Just(pkg_url);
last_path = parent.path();
parent = URL("..", &parent);
// cross-platform root check
} while (parent.path() != last_path);
return Nothing<URL>();
}

Expand All @@ -572,26 +505,27 @@ Maybe<URL> ResolveModule(Environment* env,
Maybe<URL> Resolve(Environment* env,
const std::string& specifier,
const URL& base) {
URL pure_url(specifier);
if (!(pure_url.flags() & URL_FLAGS_FAILED)) {
// just check existence, without altering
Maybe<uv_file> check = CheckFile(pure_url.ToFilePath());
if (check.IsNothing()) {
return Nothing<URL>();
// Order swapped from spec for minor perf gain.
// Ok since relative URLs cannot parse as URLs.
URL resolved;
if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
resolved = URL(specifier, base);
} else {
URL pure_url(specifier);
if (!(pure_url.flags() & URL_FLAGS_FAILED)) {
resolved = pure_url;
} else {
return PackageResolve(env, specifier, base);
}
return Just(pure_url);
}
if (specifier.length() == 0) {
DescriptorType check = CheckDescriptor(resolved.ToFilePath());
if (check != FILE) {
std::string msg = "Cannot find module '" + resolved.ToFilePath() +
"' imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}
if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
URL resolved(specifier, base);
if (ResolvesToFile(resolved))
return Just(resolved);
return Nothing<URL>();
} else {
return ResolveModule(env, specifier, base);
}
return Just(resolved);
}

void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
Expand All @@ -613,10 +547,18 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
env, "second argument is not a URL string");
}

TryCatch try_catch(env->isolate());
Maybe<URL> result = node::loader::Resolve(env, specifier_std, url);
if (result.IsNothing() || (result.FromJust().flags() & URL_FLAGS_FAILED)) {
std::string msg = "Cannot find module " + specifier_std;
return node::THROW_ERR_MISSING_MODULE(env, msg.c_str());
if (try_catch.HasCaught()) {
try_catch.ReThrow();
return;
} else if (result.IsNothing() ||
(result.FromJust().flags() & URL_FLAGS_FAILED)) {
std::string msg = "Cannot find module '" + specifier_std +
"' imported from " + url.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
try_catch.ReThrow();
return;
}

args.GetReturnValue().Set(result.FromJust().ToObject(env));
Expand Down
2 changes: 1 addition & 1 deletion src/node_errors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ void FatalException(const v8::FunctionCallbackInfo<v8::Value>& args);
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
V(ERR_MISSING_ARGS, TypeError) \
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \
V(ERR_MISSING_MODULE, Error) \
V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \
V(ERR_MODULE_NOT_FOUND, Error) \
V(ERR_OUT_OF_RANGE, RangeError) \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \
V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \
Expand Down
Loading