fix: array pattern implementation

.changeset/breezy-plants-search.md

@@ -0,0 +1,30 @@
+---
+'resolve.imports': patch
+---
+
+Fix array pattern handling.
+
+The array pattern actually does not return an array of resolved paths.
+It returns the first match with condition support.
+
+i.e.:
+
+```ts
+//=> "./a.js"
+{
+  "imports": {
+    "#a": ["./a.js", "./b.js"]
+  }
+}
+
+// `conditions: ["node"] => "./node.js"`
+// `conditions: undefined => "./browser.js"`
+{
+  "imports": {
+    "#a": [
+      { "node": "./node.js" },
+      "./browser.js"
+    ]
+  }
+}
+```

packages/resolve.imports/README.md

@@ -110,6 +110,28 @@ resolve(manifest, '#internal/a.js')
 ```ts
 import { resolve } from 'resolve.imports';
 
+const manifest = {
+  content: {
+    imports: {
+      '#internal/*.js': ['./src/internal/*.js', './src/internal2/*.js'],
+      '#utils/*.js': [{ node: './src/utils/*.js' }, './src/utils2/*.js']
+    }
+  }
+}
+
+//=> './src/internal/foo.js'
+resolve(manifest, '#internal/a.js')
+
+//=> './src/utils/foo.js'
+resolve(manifest, '#utils/a.js', { conditions: ['node'] })
+
+//=> './src/utils2/foo.js'
+resolve(manifest, '#utils/a.js')
+```
+
+```ts
+import { resolve } from 'resolve.imports';
+
 const manifest = {
   content: {
     imports: {
@@ -118,7 +140,7 @@ const manifest = {
   }
 }
 
-//=> ['./src/internal/foo.js', './src/internal2/foo.js']
+//=> './src/internal/foo.js'
 resolve(manifest, '#internal/a.js')
 ```
 

packages/resolve.imports/ts/index.ts

@@ -20,7 +20,7 @@ export type ImportsFieldManifest = {
   }
 }
 
-export type ImportMap = string | string[] | { [key: string]: ImportMap }
+export type ImportMap = string | Array<ImportMap> | { [key: string]: ImportMap }
 
 export type ResolveOptions = {
   /**
@@ -46,7 +46,7 @@ export function resolve(manifest: ImportsFieldManifest, specifier: string, optio
     const conditions = new Set(options?.conditions ?? [])
     const matched = manifest.content.imports[specifier]
     if (matched) {
-      return noRecursive(lookupReplacer(matched, conditions))
+      return noRecursive(resolvePackagePattern(matched, conditions))
     }
 
     const expansionKeys = getExpensionKeys(Object.keys(manifest.content.imports))
@@ -55,11 +55,9 @@ export function resolve(manifest: ImportsFieldManifest, specifier: string, optio
 
       const [prefix, suffix] = keyParts
       if (specifier.startsWith(prefix)) {
-        const replacer = lookupReplacer(manifest.content.imports[key], conditions)
+        const replacer = resolvePackagePattern(manifest.content.imports[key], conditions)
 
-        if (replacer) return noRecursive(
-          Array.isArray(replacer) ? replacer.map(replacePattern) : replacePattern(replacer)
-        )
+        if (replacer) return noRecursive(replacePattern(replacer))
       }
 
       function replacePattern(replacer: string) {
@@ -72,18 +70,26 @@ export function resolve(manifest: ImportsFieldManifest, specifier: string, optio
   throw new ERR_PACKAGE_IMPORT_NOT_DEFINED(specifier, manifest.path, manifest.base)
 }
 
-function lookupReplacer(map: ImportMap, conditions: Set<string>): string | string[] | undefined {
-  if (typeof map === 'string' || Array.isArray(map)) return map
+function resolvePackagePattern(map: ImportMap, conditions: Set<string>): string | undefined {
+  if (typeof map === 'string') return map
+
+  if (Array.isArray(map)) {
+    for (const item of map) {
+      const result = resolvePackagePattern(item, conditions)
+      if (result) return result
+    }
+    return undefined
+  }
 
   for (const key of Object.keys(map)) {
-    if (conditions.has(key)) return lookupReplacer(map[key], conditions)
+    if (conditions.has(key)) return resolvePackagePattern(map[key], conditions)
   }
   return map.default as string | undefined
 }
 
-function noRecursive(value: string | string[] | undefined): string | string[] | undefined {
-  if (Array.isArray(value)) return value.map(noRecursive) as string[]
-  return value?.startsWith('#') ? undefined : value
+function noRecursive(value: string | undefined): string | undefined {
+  assert(!value?.startsWith('#'), 'recursive imports are not allowed')
+  return value
 }
 
 function getExpensionKeys(keys: string[]) {

packages/resolve.imports/ts/index.spec.ts → packages/resolve.imports/ts/resolve.spec.ts

@@ -44,18 +44,14 @@ describe('subpath imports', () => {
     expect(r).toBe('./browser.js')
   })
 
-  it('returns the value if it is string[]', () => {
-    // the actual resolution (check which file exists and thus accept),
-    // is up to the engine, we just return the array.
-    const r = resolve(
-      manifest({
-        imports: {
-          '#ansi-styles': ['./a.js', './b.js']
-        }
-      }),
-      '#ansi-styles'
-    )
-    expect(r).toEqual(['./a.js', './b.js'])
+  it('supports conditional imports in array form', () => {
+    const m = manifest({
+      imports: {
+        '#ansi-styles': [{ "node": './a.js' }, './b.js']
+      }
+    })
+    expect(resolve(m, '#ansi-styles')).toEqual('./b.js')
+    expect(resolve(m, '#ansi-styles', { conditions: ['blah', 'node'] })).toEqual('./a.js')
   })
 
   it('returns first matched condition', () => {
@@ -160,7 +156,7 @@ describe(`subpath patterns`, () => {
     expect(r).toBe('./src/internal/foo.js')
   })
 
-  it('maps to string[] pattern', () => {
+  it('gets the first value from array pattern', () => {
     const r = resolve(
       manifest({
         imports: {
@@ -169,7 +165,17 @@ describe(`subpath patterns`, () => {
       }),
       '#internal/foo.js'
     )
-    expect(r).toEqual(['./src/internal/foo.js', './src/internal2/foo.js'])
+    expect(r).toEqual('./src/internal/foo.js')
+  })
+
+  it('supports conditional imports in array form', () => {
+    const m = manifest({
+      imports: {
+        '#a/*': [{ "node": './a/*.js' }, './b/*.js']
+      }
+    })
+    expect(resolve(m, '#a/foo')).toEqual('./b/foo.js')
+    expect(resolve(m, '#a/foo', { conditions: ['blah', 'node'] })).toEqual('./a/foo.js')
   })
 
   it('maps based on condition', () => {
@@ -288,6 +294,6 @@ it('does not support recursive references', () => {
       '#b': './b.js',
     }
   })
-  expect(resolve(pkg, '#a')).toBeUndefined()
-  expect(resolve(pkg, '#internal/foo.js')).toBeUndefined()
+  expect(() => resolve(pkg, '#a')).toThrow(`recursive imports are not allowed`)
+  expect(() => resolve(pkg, '#internal/foo.js')).toThrow(`recursive imports are not allowed`)
 })