Skip to content

module: package imports targets outside package #52641

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion doc/api/packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

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


The resolution rules for the imports field are otherwise analogous to the
exports field.
Expand Down
41 changes: 30 additions & 11 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@
}
const pjsonPath = fileURLToPath(pjsonUrl);
const double = RegExpPrototypeExec(doubleSlashRegEx, isTarget ? target : request) !== null;
console.trace({

Check failure on line 108 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Unexpected use of 'console'. Use `const console = require('internal/console/global');` instead of the global

Check failure on line 108 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'console' is not defined
target, request

Check failure on line 109 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing trailing comma
})

Check failure on line 110 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing semicolon
process.emitWarning(
`Use of deprecated ${double ? 'double slash' :
'leading or trailing slash matching'} resolving "${target}" for module ` +
Expand Down Expand Up @@ -355,6 +358,7 @@
* @param {boolean} internal - Whether the target is internal to the package.
* @param {boolean} isPathMap - Whether the target is a path map.
* @param {string[]} conditions - The import conditions.
* @param {boolean} mustBeInternalTarget - If target must be in the package boundary.
* @returns {URL} - The resolved URL object.
* @throws {ERR_INVALID_PACKAGE_TARGET} - If the target is invalid.
* @throws {ERR_INVALID_SUBPATH} - If the subpath is invalid.
Expand All @@ -369,14 +373,15 @@
internal,
isPathMap,
conditions,
mustBeInternalTarget = true,
) {

if (subpath !== '' && !pattern && target[target.length - 1] !== '/') {
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);
}

if (!StringPrototypeStartsWith(target, './')) {
if (internal && !StringPrototypeStartsWith(target, '../') &&
if (!StringPrototypeStartsWith(target, './') && !StringPrototypeStartsWith(target, '../')) {
if (internal &&
!StringPrototypeStartsWith(target, '/')) {
// No need to convert target to string, since it's already presumed to be
if (!URLCanParse(target)) {
Expand All @@ -390,8 +395,17 @@
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);
}

if (RegExpPrototypeExec(invalidSegmentRegEx, StringPrototypeSlice(target, 2)) !== null) {
if (RegExpPrototypeExec(deprecatedInvalidSegmentRegEx, StringPrototypeSlice(target, 2)) === null) {
// skip ../ and ./ prefixes when looking for invalid segments
const skipPrefix = target.length > 1 && target[0] === '.' ?
target[1] === '/' ?
2 :
target.length > 2 && target[1] === '.' && target[2] === '/' ?
3 :
0
:

Check failure on line 405 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Bad line breaking before and after ':'
0

Check failure on line 406 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing semicolon
if (RegExpPrototypeExec(invalidSegmentRegEx, StringPrototypeSlice(target, skipPrefix)) !== null) {
if (RegExpPrototypeExec(deprecatedInvalidSegmentRegEx, StringPrototypeSlice(target, skipPrefix)) === null) {
if (!isPathMap) {
const request = pattern ?
StringPrototypeReplace(match, '*', () => subpath) :
Expand All @@ -410,7 +424,7 @@
const resolvedPath = resolved.pathname;
const packagePath = new URL('.', packageJSONUrl).pathname;

if (!StringPrototypeStartsWith(resolvedPath, packagePath)) {
if (mustBeInternalTarget && !StringPrototypeStartsWith(resolvedPath, packagePath)) {
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);
}

Expand Down Expand Up @@ -461,14 +475,16 @@
* @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. Used to restrict
* targets to be inside of the package.json directory for "exports".
* @returns {URL | null | undefined} - The resolved target, or null if not found, or undefined if not resolvable.
*/
function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
base, pattern, internal, isPathMap, conditions) {
base, pattern, internal, isPathMap, conditions, mustBeInternalTarget) {
if (typeof target === 'string') {
return resolvePackageTargetString(
target, subpath, packageSubpath, packageJSONUrl, base, pattern, internal,
isPathMap, conditions);
isPathMap, conditions, mustBeInternalTarget);
} else if (ArrayIsArray(target)) {
if (target.length === 0) {
return null;
Expand All @@ -481,7 +497,7 @@
try {
resolveResult = resolvePackageTarget(
packageJSONUrl, targetItem, subpath, packageSubpath, base, pattern,
internal, isPathMap, conditions);
internal, isPathMap, conditions, mustBeInternalTarget);
} catch (e) {
lastException = e;
if (e.code === 'ERR_INVALID_PACKAGE_TARGET') {
Expand Down Expand Up @@ -518,7 +534,7 @@
const conditionalTarget = target[key];
const resolveResult = resolvePackageTarget(
packageJSONUrl, conditionalTarget, subpath, packageSubpath, base,
pattern, internal, isPathMap, conditions);
pattern, internal, isPathMap, conditions, mustBeInternalTarget);
if (resolveResult === undefined) { continue; }
return resolveResult;
}
Expand Down Expand Up @@ -583,6 +599,7 @@
const resolveResult = resolvePackageTarget(
packageJSONUrl, target, '', packageSubpath, base, false, false, false,
conditions,
true,
);

if (resolveResult == null) {
Expand Down Expand Up @@ -635,7 +652,7 @@
true,
false,
StringPrototypeEndsWith(packageSubpath, '/'),
conditions);
conditions, true);

if (resolveResult == null) {
throw exportsNotFound(packageSubpath, packageJSONUrl, base);
Expand Down Expand Up @@ -693,6 +710,7 @@
const resolveResult = resolvePackageTarget(
packageJSONUrl, imports[name], '', name, base, false, true, false,
conditions,
false,
);
if (resolveResult != null) {
return resolveResult;
Expand Down Expand Up @@ -725,7 +743,8 @@
const resolveResult = resolvePackageTarget(packageJSONUrl, target,
bestMatchSubpath,
bestMatch, base, true,
true, false, conditions);
true, false, conditions,
false);
if (resolveResult != null) {
return resolveResult;
}
Expand Down
6 changes: 4 additions & 2 deletions test/es-module/test-esm-imports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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' }],
// Target steps uses pattern below the package base
['#belowbase/nested', { default: 'nested' }],
]);

for (const [validSpecifier, expected] of internalImports) {
Expand All @@ -38,8 +42,6 @@ const { requireImport, importImport } = importer;
}

const invalidImportTargets = new Set([
// Target steps below the package base
['#belowbase', '#belowbase'],
// Target is a URL
['#url', '#url'],
]);
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/es-modules/belowbase.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'belowbase'
1 change: 1 addition & 0 deletions test/fixtures/es-modules/belowbase/nested.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'nested'
3 changes: 2 additions & 1 deletion test/fixtures/es-modules/pkgimports/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"#external": "pkgexports/valid-cjs",
"#external/subpath/*": "pkgexports/sub/*",
"#external/invalidsubpath/": "pkgexports/sub",
"#belowbase": "../belowbase",
"#belowbase": "../belowbase.js",
"#belowbase/*": "../belowbase/*.js",
"#url": "some:url",
"#null": null,
"#nullcondition": {
Expand Down
Loading