From e358b6d1d14107ea4e9b15a558862e875079b27b Mon Sep 17 00:00:00 2001 From: cyberuni Date: Sun, 1 Jan 2023 10:50:00 -0800 Subject: [PATCH] fix: do not support recursive resolution It will return `undefined` if the import is recursive. --- .changeset/metal-planets-end.md | 8 ++ packages/resolve.imports/README.md | 133 +++++++++++++++++++++- packages/resolve.imports/ts/index.spec.ts | 39 +++++++ packages/resolve.imports/ts/index.ts | 11 +- 4 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 .changeset/metal-planets-end.md diff --git a/.changeset/metal-planets-end.md b/.changeset/metal-planets-end.md new file mode 100644 index 0000000..04988e2 --- /dev/null +++ b/.changeset/metal-planets-end.md @@ -0,0 +1,8 @@ +--- +'resolve.imports': patch +--- + +Do not support recursive resolution. + +There is no spec for it. +It will return `undefined` if the import is recursive. diff --git a/packages/resolve.imports/README.md b/packages/resolve.imports/README.md index cf5a655..1e78ec6 100644 --- a/packages/resolve.imports/README.md +++ b/packages/resolve.imports/README.md @@ -4,7 +4,13 @@ [![NPM downloads][downloads-image]][downloads-url] [![Codecov][codecov-image]][codecov-url] -Resolves files based on the [imports] field within the specified `package.json`. +[subpath-imports] field resolver without file-system reliance. + +It uses a new logic differs from [resolve.exports] which also handles: + +- [File extensions](#subpath-imports) ([issue in `resolve.exports`][file-extensions-issue]) +- [Array patterns](#array-patterns) ([issue in `resolve.exports`][array-patterns-issue]) +- [Subpath patterns with extensions](#subpath-imports) ([issue in `resolve.exports`][subpath-patterns-issue]) ## Install @@ -24,11 +30,125 @@ rush add -p resolve.imports ## Usage +Here is the API: + +`resolve(pjson: Record, entry: string, options?: { conditions?: string[] }): string | undefined`: + +- `pjson` is the package.json object. +- `entry` is the entry to resolve. +- `options` is optional. It contains: + - `conditions` is the conditions to resolve. It is used for [subpath imports][subpath-imports]. + +It returns either a `string`, `string[]` (for [array pattern](#array-patterns)) or `undefined`. + +Note that it does not support recursive resolution. i.e.: + +```ts +import { resolve } from 'resolve.imports'; + +const pjson = { + "imports": { + "#internal/*.js": "#another-internal/*.js", + "#another-internal/*.js": "./src/another-internal/*.js" + } +} + +resolve(pjson, '#internal/foo.js') //=> undefined +``` + +It is not supported because I can't find such use case in the spec. +If you have such use case, please open an issue. + +### Subpath imports + +[Subpath imports][subpath-imports] are supported: + +Using [chalk] as an example: + +```ts +import { resolve } from 'resolve.imports'; + +const chalkPackageJson = { + "imports": { + "#ansi-styles": "./source/vendor/ansi-styles/index.js", + "#supports-color": { + "node": "./source/vendor/supports-color/index.js", + "default": "./source/vendor/supports-color/browser.js" + } + } +} + +resolve(chalkPackageJson, '#ansi-styles') //=> `./source/vendor/ansi-styles/index.js` +resolve(chalkPackageJson, '#supports-color') //=> `./source/vendor/supports-color/browser.js` +resolve(chalkPackageJson, '#supports-color', { conditions: ['node'] }) //=> `./source/vendor/supports-color/index.js` +resolve(chalkPackageJson, '#supports-color', { conditions: ['default']}) //=> `./source/vendor/supports-color/browser.js` +``` + +### File extensions + +[File extensions][file-extensions-issue] are supported: + ```ts import { resolve } from 'resolve.imports'; -// `path = './source/vendored/ansi-styles/index.js'` -const path = resolve(packageJsonContent, '#ansi-styles', { conditions: ['imports', 'node', 'default'] }); +const pjson = { + imports: { + '#internal/a.js': './src/internal/a.js', +} + +resolve(pjson, '#internal/a.js') //=> `./src/internal/a.js` +``` + +### Array patterns + +```ts +import { resolve } from 'resolve.imports'; + +const pjson = { + imports: { + '#internal/*.js': ['./src/internal/*.js', './src/internal2/*.js'] +} + +resolve(pjson, '#internal/a.js') //=> ['./src/internal/foo.js', './src/internal2/foo.js'] +``` + +### Subpath patterns + +[Subpath patterns][subpath-patterns] are supported: + +```ts +import { resolve } from 'resolve.imports'; + +const pjson = { + "imports": { + "#internal/*.js": "./src/internal/*.js" + } +} + +resolve(pjson, '#internal/foo.js') //=> `./src/internal/foo.js` +``` + +### Nested conditions + +[Nested conditions](https://nodejs.org/api/packages.html#nested-conditions) are supported: + +```ts +import { resolve } from 'resolve.imports'; + +const pjson = { + "imports": { + '#feature': { + "node": { + "import": "./feature-node.mjs", + "require": "./feature-node.cjs" + }, + "default": "./feature.mjs" + } + } +} + +resolve(pjson, '#feature') //=> `./feature.mjs` +resolve(pjson, '#feature', { conditions: ['node', 'import']}) //=> `./feature-node.mjs` ``` This is used by [@repobuddy/jest] to resolve ESM packages correctly. @@ -41,10 +161,15 @@ This is used by [@repobuddy/jest] to resolve ESM packages correctly. - [@node-loader/import-maps](https://github.com/node-loader/node-loader-import-maps) [@repobuddy/jest]: https://github.com/repobuddy/jest +[array-patterns-issue]: https://github.com/lukeed/resolve.exports/issues/17 +[chalk]: https://github.com/chalk/chalk [codecov-image]: https://codecov.io/gh/cyberuni/resolve.imports/branch/main/graph/badge.svg [codecov-url]: https://codecov.io/gh/cyberuni/resolve.imports [downloads-image]: https://img.shields.io/npm/dm/resolve.imports.svg?style=flat [downloads-url]: https://npmjs.org/package/resolve.imports -[imports]: https://nodejs.org/api/packages.html#subpath-imports +[file-extensions-issue]: https://github.com/lukeed/resolve.exports/issues/22 [npm-image]: https://img.shields.io/npm/v/resolve.imports.svg?style=flat [npm-url]: https://npmjs.org/package/resolve.imports +[subpath-imports]: https://nodejs.org/api/packages.html#subpath-imports +[subpath-patterns-issue]: https://github.com/lukeed/resolve.exports/issues/16 +[subpath-patterns]: https://nodejs.org/api/packages.html#subpath-patterns diff --git a/packages/resolve.imports/ts/index.spec.ts b/packages/resolve.imports/ts/index.spec.ts index ddfd571..d4ecbd5 100644 --- a/packages/resolve.imports/ts/index.spec.ts +++ b/packages/resolve.imports/ts/index.spec.ts @@ -100,6 +100,31 @@ describe('no * pattern', () => { ) expect(r).toBe('./node.mjs') }) + + it('works with explicit file path', () => { + const pkg = { + imports: { + '#internal/a.js': './src/internal/a.js', + '#internal/b.js': { + import: './src/internal/b.mjs', + require: './src/internal/b.cjs', + default: './src/browser/b.mjs' + }, + '#internal/c.js': { + node: { + import: './src/internal/c.mjs', + require: './src/internal/c.cjs', + }, + default: './src/browser/c.mjs' + } + } + } + expect(resolve(pkg, '#internal/a.js')).toBe('./src/internal/a.js') + expect(resolve(pkg, '#internal/b.js')).toBe('./src/browser/b.mjs') + expect(resolve(pkg, '#internal/b.js', { conditions: ['import'] })).toBe('./src/internal/b.mjs') + expect(resolve(pkg, '#internal/c.js')).toBe('./src/browser/c.mjs') + expect(resolve(pkg, '#internal/c.js', { conditions: ['node', 'require'] })).toBe('./src/internal/c.cjs') + }) }) describe(`subpath patterns`, () => { @@ -209,4 +234,18 @@ describe(`subpath patterns`, () => { ) expect(r).toBe('./src/internal/foo/foo.js') }) + + // Do not see any spec for this. + // So do not support it for now to avoid deviating from spec. + it('does not support recursive references', () => { + const pkg = { + imports: { + '#internal/*.js': '#internal/*.js', + '#a': '#b', + '#b': './b.js', + } + } + expect(resolve(pkg, '#a')).toBeUndefined() + expect(resolve(pkg, '#internal/foo.js')).toBeUndefined() + }) }) diff --git a/packages/resolve.imports/ts/index.ts b/packages/resolve.imports/ts/index.ts index f16bb53..2e89210 100644 --- a/packages/resolve.imports/ts/index.ts +++ b/packages/resolve.imports/ts/index.ts @@ -18,7 +18,7 @@ export function resolve(pkg: any, entry: string, options?: ResolveOptions) { const matched = pkg.imports[entry] if (matched) { - return lookupReplacer(matched, options?.conditions?.slice()) + return noRecursive(lookupReplacer(matched, options?.conditions?.slice())) } for (const key in pkg.imports) { @@ -28,7 +28,9 @@ export function resolve(pkg: any, entry: string, options?: ResolveOptions) { if (entry.startsWith(prefix)) { const replacer = lookupReplacer(pkg.imports[key], options?.conditions?.slice()) - if (replacer) return Array.isArray(replacer) ? replacer.map(replacePattern) : replacePattern(replacer) + if (replacer) return noRecursive( + Array.isArray(replacer) ? replacer.map(replacePattern) : replacePattern(replacer) + ) } function replacePattern(replacer: string) { @@ -51,3 +53,8 @@ function lookupReplacer(map: ImportMap, conditions?: string[]): string | string[ } return map.default as any } + +function noRecursive(value: string | string[] | undefined): string | string[] | undefined { + if (Array.isArray(value)) return value.map(noRecursive) as string[] + return value?.startsWith('#') ? undefined : value +}