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: exports patterns #34718

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
49 changes: 35 additions & 14 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,8 @@ The resolver can throw the following errors:
> 1. Set _mainExport_ to _exports_\[_"."_\].
> 1. If _mainExport_ is not **undefined**, then
> 1. Let _resolved_ be the result of **PACKAGE_TARGET_RESOLVE**(
> _packageURL_, _mainExport_, _""_, **false**, _conditions_).
> _packageURL_, _mainExport_, _""_, **false**, **false**,
> _conditions_).
> 1. If _resolved_ is not **null** or **undefined**, then
> 1. Return _resolved_.
> 1. Otherwise, if _exports_ is an Object and all keys of _exports_ start with
Expand Down Expand Up @@ -1010,29 +1011,43 @@ _isImports_, _conditions_)
> 1. If _matchKey_ is a key of _matchObj_, and does not end in _"*"_, then
> 1. Let _target_ be the value of _matchObj_\[_matchKey_\].
> 1. Let _resolved_ be the result of **PACKAGE_TARGET_RESOLVE**(
> _packageURL_, _target_, _""_, _isImports_, _conditions_).
> _packageURL_, _target_, _""_, **false**, _isImports_, _conditions_).
> 1. Return the object _{ resolved, exact: **true** }_.
> 1. Let _expansionKeys_ be the list of keys of _matchObj_ ending in _"/"_,
> sorted by length descending.
> 1. Let _expansionKeys_ be the list of keys of _matchObj_ ending in _"/"_
> or _"*"_, sorted by length descending.
> 1. For each key _expansionKey_ in _expansionKeys_, do
> 1. If _expansionKey_ ends in _"*"_ and _matchKey_ starts with but is
> not equal to the substring of _expansionKey_ excluding the last _"*"_
> character, then
> 1. Let _target_ be the value of _matchObj_\[_expansionKey_\].
> 1. Let _subpath_ be the substring of _matchKey_ starting at the
> index of the length of _expansionKey_ minus one.
> 1. Let _resolved_ be the result of **PACKAGE_TARGET_RESOLVE**(
> _packageURL_, _target_, _subpath_, **true**, _isImports_,
> _conditions_).
> 1. Return the object _{ resolved, exact: **true** }_.
> 1. If _matchKey_ starts with _expansionKey_, then
> 1. Let _target_ be the value of _matchObj_\[_expansionKey_\].
> 1. Let _subpath_ be the substring of _matchKey_ starting at the
> index of the length of _expansionKey_.
> 1. Let _resolved_ be the result of **PACKAGE_TARGET_RESOLVE**(
> _packageURL_, _target_, _subpath_, _isImports_, _conditions_).
> _packageURL_, _target_, _subpath_, **false**, _isImports_,
> _conditions_).
> 1. Return the object _{ resolved, exact: **false** }_.
> 1. Return the object _{ resolved: **null**, exact: **true** }_.

**PACKAGE_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_, _internal_,
_conditions_)
**PACKAGE_TARGET_RESOLVE**(_packageURL_, _target_, _subpath_, _pattern_,
_internal_, _conditions_)

> 1. If _target_ is a String, then
> 1. If _subpath_ has non-zero length and _target_ does not end with _"/"_,
> throw an _Invalid Module Specifier_ error.
> 1. If _pattern_ is **false**, _subpath_ has non-zero length and _target_
> does not end with _"/"_, throw an _Invalid Module Specifier_ error.
> 1. If _target_ does not start with _"./"_, then
> 1. If _internal_ is **true** and _target_ does not start with _"../"_ or
> _"/"_ and is not a valid URL, then
> 1. If _pattern_ is **true**, then
> 1. Return **PACKAGE_RESOLVE**(_target_ with every instance of
> _"*"_ replaced by _subpath_, _packageURL_ + _"/"_)_.
> 1. Return **PACKAGE_RESOLVE**(_target_ + _subpath_,
> _packageURL_ + _"/"_)_.
> 1. Otherwise, throw an _Invalid Package Target_ error.
Expand All @@ -1044,8 +1059,12 @@ _conditions_)
> 1. Assert: _resolvedTarget_ is contained in _packageURL_.
> 1. If _subpath_ split on _"/"_ or _"\\"_ contains any _"."_, _".."_ or
> _"node_modules"_ segments, throw an _Invalid Module Specifier_ error.
> 1. Return the URL resolution of the concatenation of _subpath_ and
> _resolvedTarget_.
> 1. If _pattern_ is **true**, then
> 1. Return the URL resolution of _resolvedTarget_ with every instance of
> _"*"_ replaced with _subpath_.
> 1. Otherwise,
> 1. Return the URL resolution of the concatenation of _subpath_ and
> _resolvedTarget_.
> 1. Otherwise, if _target_ is a non-null Object, then
> 1. If _exports_ contains any index property keys, as defined in ECMA-262
> [6.1.7 Array Index][], throw an _Invalid Package Configuration_ error.
Expand All @@ -1054,16 +1073,18 @@ _conditions_)
> then
> 1. Let _targetValue_ be the value of the _p_ property in _target_.
> 1. Let _resolved_ be the result of **PACKAGE_TARGET_RESOLVE**(
> _packageURL_, _targetValue_, _subpath_, _internal_, _conditions_).
> _packageURL_, _targetValue_, _subpath_, _pattern_, _internal_,
> _conditions_).
> 1. If _resolved_ is equal to **undefined**, continue the loop.
> 1. Return _resolved_.
> 1. Return **undefined**.
> 1. Otherwise, if _target_ is an Array, then
> 1. If _target.length is zero, return **null**.
> 1. For each item _targetValue_ in _target_, do
> 1. Let _resolved_ be the result of **PACKAGE_TARGET_RESOLVE**(
> _packageURL_, _targetValue_, _subpath_, _internal_, _conditions_),
> continuing the loop on any _Invalid Package Target_ error.
> _packageURL_, _targetValue_, _subpath_, _pattern_, _internal_,
> _conditions_), continuing the loop on any _Invalid Package Target_
> error.
> 1. If _resolved_ is **undefined**, continue the loop.
> 1. Return _resolved_.
> 1. Return or throw the last fallback resolution **null** return or error.
Expand Down
43 changes: 30 additions & 13 deletions doc/api/packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,17 @@ Alternatively a project could choose to export entire folders:
"exports": {
".": "./lib/index.js",
"./lib": "./lib/index.js",
"./lib/": "./lib/",
"./lib/*": "./lib/*.js",
"./feature": "./feature/index.js",
"./feature/": "./feature/",
"./feature/*": "./feature/*.js",
"./package.json": "./package.json"
}
}
```

As a last resort, package encapsulation can be disabled entirely by creating an
export for the root of the package `"./": "./"`. This will expose every file in
the package at the cost of disabling the encapsulation and potential tooling
export for the root of the package `"./*": "./*"`. This will expose every file
in the package at the cost of disabling the encapsulation and potential tooling
benefits this provides. As the ES Module loader in Node.js enforces the use of
[the full specifier path][], exporting the root rather than being explicit
about entry is less expressive than either of the prior examples. Not only
Expand Down Expand Up @@ -289,29 +289,46 @@ import submodule from 'es-module-package/private-module.js';
// Throws ERR_PACKAGE_PATH_NOT_EXPORTED
```

Entire folders can also be mapped with package exports:
### Subpath export patterns

> Stability: 1 - Experimental

Explicitly listing each exports subpath entry is recommended for packages with
a small number of exports. But for packages that have very large numbers of
subpaths this can start to cause package.json bloat and maintenanec issues.

For these use cases, subpath export patterns can be used instead:

```json
// ./node_modules/es-module-package/package.json
{
"exports": {
"./features/": "./src/features/"
"./features/*": "./src/features/*.js"
Copy link
Member

Choose a reason for hiding this comment

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

Confirming: * matches / as well? Would maybe we want to make * not match /, and ** match /? That way you could control whether you had nested patterns or not.

Copy link
Contributor

@jkrems jkrems Sep 17, 2020

Choose a reason for hiding this comment

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

I think I would rather make it (.*) since that's the other common (and more modern) way to write "splats" in route definitions. The current syntax matches what express <=4 (?) used. I fee like using glob-style patterns would imply expansion, not matching/1-to-1 mapping.

Copy link
Member

Choose a reason for hiding this comment

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

These are file paths, not route definitions, in which double-star globbing is exceedingly common. Why would express routing patterns be appropriate here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exports patterns are not file paths, they are effectively a relative URL space (including URI encoding) that apply a mapping to a file space, so routes are a fairly good analogy.

Copy link
Member

Choose a reason for hiding this comment

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

If it's going to match express' routing DSL, then that's a different story (including path segment awareness), but I don't think it would make sense to only partially mimic express.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ljharb * will match any separators yes. But if anything the right hand side is more what should be thought of as a glob since replacing the * in the right hand side with ** is what will give you the file system glob to enumerate the exports of the package.

Your comment also triggered another question though which I hadn't considered. And that is that:

{
  "exports": {
    "./*": "./*"
  }
}

is clearly defined to only match explicit subpaths of the package. That is, import.meta.resolve('pkg/') would not resolve that export mapping (currently it throws with this PR, unlike "./") and therefore, interestingly, if ./ path mappings are deprecated could this could be extended to always resolve the package base in a subsequent PR if we want to.

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, why would that be different? A * can always be empty.

Copy link
Contributor Author

@guybedford guybedford Sep 17, 2020

Choose a reason for hiding this comment

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

This PR explicitly requires some text to be matched on the LHS for the *. If you write:

{
  "exports": {
    "./components-*": "./components/*.js"
  }
}

it will match "pkg/components-x" but "pkg/components-" will throw a package path not exported error instead of trying to map into components/.js and throwing a not found.

Copy link
Member

Choose a reason for hiding this comment

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

That seems very confusing, and not how * works in literally every other context everywhere.

}
}
```

With the preceding, all modules within the `./src/features/` folder
are exposed deeply to `import` and `require`:
The left hand matching pattern must always end in `*`, and all instances of `*`
guybedford marked this conversation as resolved.
Show resolved Hide resolved
on the right hand side will then be replaced with this value, including if it
contains any `/` separators.

```js
import feature from 'es-module-package/features/x.js';
import featureX from 'es-module-package/features/x';
// Loads ./node_modules/es-module-package/src/features/x.js

import featureY from 'es-module-package/features/y/y';
// Loads ./node_modules/es-module-package/src/features/y/y.js
```

When using folder mappings, ensure that you do want to expose every
module inside the subfolder. Any modules which are not public
should be moved to another folder to retain the encapsulation
benefits of exports.
This is a direct static replacement without any special handling for file
extensions. In the previous example, `pkg/features/x.json` would be resolved to
`./src/features/x.json.js` in the mapping.

The property of exports being statically enumerable is maintained with exports
patterns since the individual exports for a package can be determined by
treating the right hand side target pattern as a `**` glob against the list of
files within the package. Because `node_modules` paths are forbidden in exports
targets, this expansion is dependent on only the files of the package itself.

### Package exports fallbacks

Expand Down
64 changes: 45 additions & 19 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,11 @@ function throwInvalidPackageTarget(
}

const invalidSegmentRegEx = /(^|\\|\/)(\.\.?|node_modules)(\\|\/|$)/;
const patternRegEx = /\*/g;

function resolvePackageTargetString(
target, subpath, match, packageJSONUrl, base, internal, conditions) {
if (subpath !== '' && target[target.length - 1] !== '/')
target, subpath, match, packageJSONUrl, base, pattern, internal, conditions) {
if (subpath !== '' && !pattern && target[target.length - 1] !== '/')
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);

if (!StringPrototypeStartsWith(target, './')) {
Expand All @@ -321,8 +322,12 @@ function resolvePackageTargetString(
new URL(target);
isURL = true;
} catch {}
if (!isURL)
return packageResolve(target + subpath, packageJSONUrl, conditions);
if (!isURL) {
const exportTarget = pattern ?
StringPrototypeReplace(target, patternRegEx, subpath) :
target + subpath;
return packageResolve(exportTarget, packageJSONUrl, conditions);
}
}
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
}
Expand All @@ -342,6 +347,9 @@ function resolvePackageTargetString(
if (RegExpPrototypeTest(invalidSegmentRegEx, subpath))
throwInvalidSubpath(match + subpath, packageJSONUrl, internal, base);

if (pattern)
return new URL(StringPrototypeReplace(resolved.href, patternRegEx,
subpath));
return new URL(subpath, resolved);
}

Expand All @@ -356,10 +364,10 @@ function isArrayIndex(key) {
}

function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
base, internal, conditions) {
base, pattern, internal, conditions) {
if (typeof target === 'string') {
return resolvePackageTargetString(
target, subpath, packageSubpath, packageJSONUrl, base, internal,
target, subpath, packageSubpath, packageJSONUrl, base, pattern, internal,
conditions);
} else if (ArrayIsArray(target)) {
if (target.length === 0)
Expand All @@ -371,8 +379,8 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
let resolved;
try {
resolved = resolvePackageTarget(
packageJSONUrl, targetItem, subpath, packageSubpath, base, internal,
conditions);
packageJSONUrl, targetItem, subpath, packageSubpath, base, pattern,
internal, conditions);
} catch (e) {
lastException = e;
if (e.code === 'ERR_INVALID_PACKAGE_TARGET')
Expand Down Expand Up @@ -406,7 +414,7 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
const conditionalTarget = target[key];
const resolved = resolvePackageTarget(
packageJSONUrl, conditionalTarget, subpath, packageSubpath, base,
internal, conditions);
pattern, internal, conditions);
if (resolved === undefined)
continue;
return resolved;
Expand Down Expand Up @@ -460,7 +468,7 @@ function packageExportsResolve(
if (ObjectPrototypeHasOwnProperty(exports, packageSubpath)) {
const target = exports[packageSubpath];
const resolved = resolvePackageTarget(
packageJSONUrl, target, '', packageSubpath, base, false, conditions
packageJSONUrl, target, '', packageSubpath, base, false, false, conditions
);
if (resolved === null || resolved === undefined)
throwExportsNotFound(packageSubpath, packageJSONUrl, base);
Expand All @@ -471,7 +479,13 @@ function packageExportsResolve(
const keys = ObjectGetOwnPropertyNames(exports);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (key[key.length - 1] === '/' &&
if (key[key.length - 1] === '*' &&
StringPrototypeStartsWith(packageSubpath,
StringPrototypeSlice(key, 0, -1)) &&
packageSubpath.length >= key.length &&
key.length > bestMatch.length) {
bestMatch = key;
} else if (key[key.length - 1] === '/' &&
StringPrototypeStartsWith(packageSubpath, key) &&
key.length > bestMatch.length) {
bestMatch = key;
Expand All @@ -480,12 +494,15 @@ function packageExportsResolve(

if (bestMatch) {
const target = exports[bestMatch];
const subpath = StringPrototypeSubstr(packageSubpath, bestMatch.length);
const pattern = bestMatch[bestMatch.length - 1] === '*';
const subpath = StringPrototypeSubstr(packageSubpath, bestMatch.length -
(pattern ? 1 : 0));
const resolved = resolvePackageTarget(packageJSONUrl, target, subpath,
bestMatch, base, false, conditions);
bestMatch, base, pattern, false,
conditions);
if (resolved === null || resolved === undefined)
throwExportsNotFound(packageSubpath, packageJSONUrl, base);
return { resolved, exact: false };
return { resolved, exact: pattern };
}

throwExportsNotFound(packageSubpath, packageJSONUrl, base);
Expand All @@ -504,7 +521,7 @@ function packageImportsResolve(name, base, conditions) {
if (imports) {
if (ObjectPrototypeHasOwnProperty(imports, name)) {
const resolved = resolvePackageTarget(
packageJSONUrl, imports[name], '', name, base, true, conditions
packageJSONUrl, imports[name], '', name, base, false, true, conditions
);
if (resolved !== null)
return { resolved, exact: true };
Expand All @@ -513,7 +530,13 @@ function packageImportsResolve(name, base, conditions) {
const keys = ObjectGetOwnPropertyNames(imports);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (key[key.length - 1] === '/' &&
if (key[key.length - 1] === '*' &&
StringPrototypeStartsWith(name,
StringPrototypeSlice(key, 0, -1)) &&
name.length >= key.length &&
key.length > bestMatch.length) {
bestMatch = key;
} else if (key[key.length - 1] === '/' &&
StringPrototypeStartsWith(name, key) &&
key.length > bestMatch.length) {
bestMatch = key;
Expand All @@ -522,11 +545,14 @@ function packageImportsResolve(name, base, conditions) {

if (bestMatch) {
const target = imports[bestMatch];
const subpath = StringPrototypeSubstr(name, bestMatch.length);
const pattern = bestMatch[bestMatch.length - 1] === '*';
const subpath = StringPrototypeSubstr(name, bestMatch.length -
(pattern ? 1 : 0));
const resolved = resolvePackageTarget(
packageJSONUrl, target, subpath, bestMatch, base, true, conditions);
packageJSONUrl, target, subpath, bestMatch, base, pattern, true,
conditions);
if (resolved !== null)
return { resolved, exact: false };
return { resolved, exact: pattern };
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
{ default: 'self-cjs' } : { default: 'self-mjs' }],
// Resolve self sugar
['pkgexports-sugar', { default: 'main' }],
// Path patterns
['pkgexports/subpath/sub-dir1', { default: 'main' }],
['pkgexports/features/dir1', { default: 'main' }]
]);

if (isRequire) {
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/es-modules/pkgimports/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
"import": "./importbranch.js",
"require": "./requirebranch.js"
},
"#subpath/": "./sub/",
"#subpath/*": "./sub/*",
"#external": "pkgexports/valid-cjs",
"#external/subpath/": "pkgexports/sub/",
"#external/subpath/*": "pkgexports/sub/*",
"#external/invalidsubpath/": "pkgexports/sub",
"#belowbase": "../belowbase",
"#url": "some:url",
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/node_modules/pkgexports/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.