module: package imports targets outside package #52641
Conversation
this allows the imports field of package.json to target a location outside of the package boundary. doing so allows for imports to work in directories just altering things like the type field of package.json and to enable monorepo workspace workflows.
|
Review requested:
|
nodejs-github-bot
added
esm
| @@ -485,7 +485,7 @@ where `import '#dep'` does not get the resolution of the external package | |||
| file `./dep-polyfill.js` relative to the package in other environments. | |||
|
|
|||
| Unlike the `"exports"` field, the `"imports"` field permits mapping to external | |||
| packages. | |||
| packages and locations. | |||
There was a problem hiding this comment.
Does https://nodejs.org/api/esm.html#resolution-algorithm-specification need to be updated as well?
| @@ -414,7 +416,8 @@ function resolvePackageTargetString( | |||
| const resolvedPath = resolved.pathname; | |||
| const packagePath = new URL('.', packageJSONUrl).pathname; | |||
|
|
|||
| if (!StringPrototypeStartsWith(resolvedPath, packagePath)) { | |||
| // if (mustBeInternalTarget && !StringPrototypeStartsWith(resolvedPath, packagePath)) { | |||
There was a problem hiding this comment.
Unsure why this is here.
| // if (mustBeInternalTarget && !StringPrototypeStartsWith(resolvedPath, packagePath)) { |
| @@ -465,14 +468,15 @@ function isArrayIndex(key) { | |||
| * @param {boolean} internal - Whether the package is internal. | |||
| * @param {boolean} isPathMap - Whether the package is a path map. | |||
| * @param {Set<string>} conditions - The conditions to match. | |||
| * @param {boolean} mustBeInternalTarget - If the target must be in the package boundary. | |||
There was a problem hiding this comment.
This might be a bit much for a JSDoc comment, but is it possible to briefly explain when we would want this restriction? Is it like If the target must be in the package scope: yes for "exports", no for "imports"?
| @@ -26,6 +26,10 @@ const { requireImport, importImport } = importer; | |||
| ['#subpath//asdf.asdf', { default: 'test' }], | |||
| // Double slash | |||
| ['#subpath/as//df.asdf', { default: 'test' }], | |||
| // Target steps below the package base | |||
| ['#belowbase', { default: 'belowbase' }], | |||
There was a problem hiding this comment.
I know this is what it was called previously, but why is this “below” the base and not above the base?
There was a problem hiding this comment.
@RafaelGSS does this needs to do anything specific for permissions?
| @@ -587,6 +591,7 @@ function packageExportsResolve( | |||
| const resolveResult = resolvePackageTarget( | |||
| packageJSONUrl, target, '', packageSubpath, base, false, false, false, | |||
| conditions, | |||
| true, | |||
There was a problem hiding this comment.
I know it's not new in this PR, but I'm not a big fan of bare boolean arguments like this where someone can't tell at a glance what the true or false means when looking at the callsite. For this, adding comments like... the following example would be helpful:
const resolveResult = resolvePackageTarget(packageJSONUrl, target, '', packageSubpath, base,
false /* pattern */,
false /* internal */,
false /* isPathMap */,
conditions,
true /* mustBeInternalTarget */);
But not blocking for this PR.
this allows the imports field of package.json to target a location outside of the package boundary. doing so allows for imports to work in directories just altering things like the type field of package.json and to enable monorepo workspace workflows.
Node already allows resolving to external packages for
"imports"; this fixes some usability problems of mixing monorepos and directories containing a{"type":"module"}(or vice versa)package.jsonpreventing the ability to properly use"imports"to resolve to reasonable locations. It does not enable..in a specifiers to escape still per the tests it must be in the target of thepackage.jsonto allow escaping.An example of a case where this feature is high friction is:
Note: this still requires duplication of imports for this use case with this PR.
Another example is in a monorepo situation: