Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: self resolve bug fixes and esm ordering #31009

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
strict self resolution opt-in
  • Loading branch information
guybedford committed Dec 19, 2019
commit 8cca5c9b4f22c9626c0a3c8d91788dfdc1e00c7a
16 changes: 12 additions & 4 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,8 @@ _defaultEnv_ is the conditional environment name priority array,
> encoded strings for _"/"_ or _"\\"_, then
> 1. Throw an _Invalid Specifier_ error.
> 1. Set _selfUrl_ to the result of
guybedford marked this conversation as resolved.
Show resolved Hide resolved
> **SELF_REFERENCE_RESOLE**(_packageName_, _packageSubpath_, _parentURL_).
> **SELF_REFERENCE_RESOLE**(_packageName_, _packageSubpath_, _parentURL_,
guybedford marked this conversation as resolved.
Show resolved Hide resolved
> **true**).
> 1. If _selfUrl_ isn't empty, return _selfUrl_.
> 1. If _packageSubpath_ is _undefined_ and _packageName_ is a Node.js builtin
> module, then
Expand All @@ -1230,14 +1231,21 @@ _defaultEnv_ is the conditional environment name priority array,
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_,
> _packageSubpath_, _pjson.exports_).
> 1. Return the URL resolution of _packageSubpath_ in _packageURL_.
> 1. Set _selfUrl_ to the result of
> **SELF_REFERENCE_RESOLE**(_packageName_, _packageSubpath_, _parentURL_,
guybedford marked this conversation as resolved.
Show resolved Hide resolved
> **false**).
> 1. If _selfUrl_ isn't empty, return _selfUrl_.
> 1. Throw a _Module Not Found_ error.

**SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_)
**SELF_REFERENCE_RESOLVE**(_packageName_, _packageSubpath_, _parentURL_,
_encapsulated_)

> 1. Let _packageURL_ be the result of **READ_PACKAGE_SCOPE**(_parentURL_).
> 1. If _packageURL_ is **null**, then
> 1. Return an empty result.
> 1. Return **undefined**.
> 1. Let _pjson_ be the result of **READ_PACKAGE_JSON**(_packageURL_).
> 1. If _encapsulated_ is **true** and _pjson_ does not include an
> _"exports"_ property, then return **undefined**.
> 1. If _pjson.name_ is equal to _packageName_, then
> 1. If _packageSubpath_ is _undefined_, then
> 1. Return the result of **PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_).
Expand All @@ -1248,7 +1256,7 @@ _defaultEnv_ is the conditional environment name priority array,
> 1. Return **PACKAGE_EXPORTS_RESOLVE**(_packageURL_, _subpath_,
> _pjson.exports_).
> 1. Return the URL resolution of _subpath_ in _packageURL_.
> 1. Otherwise return an empty result.
> 1. Otherwise, return **undefined**.

**PACKAGE_MAIN_RESOLVE**(_packageURL_, _pjson_)

Expand Down
16 changes: 9 additions & 7 deletions doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,10 @@ require(X) from module at path Y
a. LOAD_AS_FILE(Y + X)
b. LOAD_AS_DIRECTORY(Y + X)
c. THROW "not found"
4. LOAD_NODE_MODULES(X, dirname(Y))
5. LOAD_SELF_REFERENCE(X, dirname(Y))
6. THROW "not found"
4. LOAD_SELF_REFERENCE(X, dirname(Y), true)
5. LOAD_NODE_MODULES(X, dirname(Y))
6. LOAD_SELF_REFERENCE(X, dirname(Y), false)
7. THROW "not found"

LOAD_AS_FILE(X)
1. If X is a file, load X as JavaScript text. STOP
Expand Down Expand Up @@ -203,11 +204,12 @@ NODE_MODULES_PATHS(START)
d. let I = I - 1
5. return DIRS

LOAD_SELF_REFERENCE(X, START)
LOAD_SELF_REFERENCE(X, START, ENCAPSULATED)
1. Find the closest package scope to START.
2. If no scope was found, throw "not found".
3. If the name in `package.json` isn't a prefix of X, throw "not found".
4. Otherwise, resolve the remainder of X relative to this package as if it
2. If no scope was found, return.
3. If ENCAPSULATED is true and the `package.json` has no "exports", return.
4. If the name in `package.json` isn't a prefix of X, throw "not found".
5. Otherwise, resolve the remainder of X relative to this package as if it
was loaded via `LOAD_NODE_MODULES` with a name in `package.json`.
```

Expand Down
16 changes: 13 additions & 3 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -431,13 +431,13 @@ function resolveBasePath(basePath, exts, isMain, trailingSlash, request) {
return filename;
}

function trySelf(parentPath, isMain, request) {
function trySelf(parentPath, isMain, request, encapsulated) {
if (!experimentalSelf) {
return false;
}

const { data: pkg, path: basePath } = readPackageScope(parentPath) || {};
if (!pkg) return false;
if (!pkg || (encapsulated && 'exports' in pkg === false)) return false;
if (typeof pkg.name !== 'string') return false;

let expansion;
Expand Down Expand Up @@ -1003,7 +1003,17 @@ Module._resolveFilename = function(request, parent, isMain, options) {
}

// Look up the filename first, since that's the cache key.
const filename = Module._findPath(request, paths, isMain);
if (parent && parent.filename) {
const filename = trySelf(parent.filename, isMain, request, true);
if (filename) {
emitExperimentalWarning('Package name self resolution');
const cacheKey = request + '\x00' +
(paths.length === 1 ? paths[0] : paths.join('\x00'));
Module._pathCache[cacheKey] = filename;
return filename;
}
}
const filename = Module._findPath(request, paths, isMain, false);
if (filename) return filename;
if (parent && parent.filename) {
const filename = trySelf(parent.filename, isMain, request);
Expand Down
12 changes: 10 additions & 2 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,8 @@ Maybe<URL> PackageExportsResolve(Environment* env,
Maybe<URL> ResolveSelf(Environment* env,
const std::string& pkg_name,
const std::string& pkg_subpath,
const URL& base) {
const URL& base,
bool encapsulated) {
if (!env->options()->experimental_resolve_self) {
return Nothing<URL>();
}
Expand All @@ -1172,6 +1173,7 @@ Maybe<URL> ResolveSelf(Environment* env,
}
}
if (!found_pjson || pcfg->name != pkg_name) return Nothing<URL>();
if (encapsulated && pcfg->exports.IsEmpty()) return Nothing<URL>();
if (!pkg_subpath.length()) {
return PackageMainResolve(env, pjson_url, *pcfg, base);
} else {
Expand Down Expand Up @@ -1227,7 +1229,7 @@ Maybe<URL> PackageResolve(Environment* env,
pkg_subpath = "." + specifier.substr(sep_index);
}

Maybe<URL> self_url = ResolveSelf(env, pkg_name, pkg_subpath, base);
Maybe<URL> self_url = ResolveSelf(env, pkg_name, pkg_subpath, base, true);
if (self_url.IsJust()) {
ProcessEmitExperimentalWarning(env, "Package name self resolution");
return self_url;
Expand Down Expand Up @@ -1266,6 +1268,12 @@ Maybe<URL> PackageResolve(Environment* env,
// Cross-platform root check.
} while (pjson_path.length() != last_path.length());

self_url = ResolveSelf(env, pkg_name, pkg_subpath, base, false);
if (self_url.IsJust()) {
ProcessEmitExperimentalWarning(env, "Package name self resolution");
return self_url;
}

std::string msg = "Cannot find package '" + pkg_name +
"' imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
Expand Down