fix(npm-resolver): request full metadata for optional dependencies (#10455)

close #9950

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/optional-deps-full-metadata.md

@@ -0,0 +1,8 @@
+---
+"@pnpm/npm-resolver": patch
+"@pnpm/fetching-types": patch
+"@pnpm/package-requester": patch
+"pnpm": patch
+---
+
+Fixed optional dependencies to request full metadata from the registry to get the `libc` field, which is required for proper platform compatibility checks [#9950](https://github.com/pnpm/pnpm/issues/9950).

.changeset/refactor-fullmetadata-options.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/fetch": major
+---
+
+Refactored `fullMetadata` option handling. The `fullMetadata` option is no longer accepted by `createFetchFromRegistry()` at construction time - it should only be passed at call time via the fetch options.

network/fetch/src/fetchFromRegistry.ts

@@ -35,7 +35,6 @@ export function fetchWithAgent (url: RequestInfo, opts: FetchWithAgentOptions):
 export type { AgentOptions }
 
 export interface CreateFetchFromRegistryOptions extends AgentOptions {
-  fullMetadata?: boolean
   userAgent?: string
   sslConfigs?: Record<string, SslConfig>
 }
@@ -46,7 +45,7 @@ export function createFetchFromRegistry (defaultOpts: CreateFetchFromRegistryOpt
       'user-agent': USER_AGENT,
       ...getHeaders({
         auth: opts?.authHeaderValue,
-        fullMetadata: defaultOpts.fullMetadata,
+        fullMetadata: opts?.fullMetadata,
         userAgent: defaultOpts.userAgent,
       }),
     }

network/fetch/test/fetchFromRegistry.test.ts

@@ -16,8 +16,8 @@ test('fetchFromRegistry', async () => {
 })
 
 test('fetchFromRegistry fullMetadata', async () => {
-  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
-  const res = await fetchFromRegistry('https://registry.npmjs.org/is-positive')
+  const fetchFromRegistry = createFetchFromRegistry({})
+  const res = await fetchFromRegistry('https://registry.npmjs.org/is-positive', { fullMetadata: true })
   const metadata = await res.json() as any // eslint-disable-line
   expect(metadata.name).toEqual('is-positive')
   expect(metadata.versions['1.0.0'].scripts).toBeTruthy()
@@ -33,7 +33,7 @@ test('authorization headers are removed before redirection if the target is on a
     .get('/is-positive')
     .reply(200, { ok: true })
 
-  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
+  const fetchFromRegistry = createFetchFromRegistry({})
   const res = await fetchFromRegistry(
     'http://registry.pnpm.io/is-positive',
     { authHeaderValue: 'Bearer 123' }
@@ -55,7 +55,7 @@ test('authorization headers are not removed before redirection if the target is
     .get('/is-positive-new')
     .reply(200, { ok: true })
 
-  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
+  const fetchFromRegistry = createFetchFromRegistry({})
   const res = await fetchFromRegistry(
     'http://registry.pnpm.io/is-positive',
     { authHeaderValue: 'Bearer 123' }
@@ -66,7 +66,7 @@ test('authorization headers are not removed before redirection if the target is
 })
 
 test('switch to the correct agent for requests on redirect from http: to https:', async () => {
-  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
+  const fetchFromRegistry = createFetchFromRegistry({})
 
   // We can test this on any endpoint that redirects from http: to https:
   const { status } = await fetchFromRegistry('http://pnpm.io/pnpm.js')
@@ -138,3 +138,70 @@ test('fail if the client certificate is not provided', async () => {
   }
   expect(err?.code).toMatch(/ECONNRESET|ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED/)
 })
+
+test('redirect to protocol-relative URL', async () => {
+  nock('http://registry.pnpm.io/')
+    .get('/foo')
+    .reply(302, '', { location: '//registry.other.org/foo' })
+  nock('http://registry.other.org/')
+    .get('/foo')
+    .reply(200, { ok: true })
+
+  const fetchFromRegistry = createFetchFromRegistry({})
+  const res = await fetchFromRegistry(
+    'http://registry.pnpm.io/foo'
+  )
+
+  expect(await res.json()).toStrictEqual({ ok: true })
+  expect(nock.isDone()).toBeTruthy()
+})
+
+test('redirect to relative URL', async () => {
+  nock('http://registry.pnpm.io/')
+    .get('/bar/baz')
+    .reply(302, '', { location: '../foo' })
+  nock('http://registry.pnpm.io/')
+    .get('/foo')
+    .reply(200, { ok: true })
+
+  const fetchFromRegistry = createFetchFromRegistry({})
+  const res = await fetchFromRegistry(
+    'http://registry.pnpm.io/bar/baz'
+  )
+
+  expect(await res.json()).toStrictEqual({ ok: true })
+  expect(nock.isDone()).toBeTruthy()
+})
+
+test('redirect to relative URL when request pkg.pr.new link', async () => {
+  nock('https://pkg.pr.new/')
+    .get('/vue@14175')
+    .reply(302, '', { location: '/vuejs/core/vue@14182' })
+
+  nock('https://pkg.pr.new/')
+    .get('/vuejs/core/vue@14182')
+    .reply(302, '', { location: '/vuejs/core/vue@82a13bb6faaa9f77a06b57e69e0934b9f620f333' })
+
+  nock('https://pkg.pr.new/')
+    .get('/vuejs/core/vue@82a13bb6faaa9f77a06b57e69e0934b9f620f333')
+    .reply(200, { ok: true })
+
+  const fetchFromRegistry = createFetchFromRegistry({})
+  const res = await fetchFromRegistry(
+    'https://pkg.pr.new/vue@14175'
+  )
+
+  expect(await res.json()).toStrictEqual({ ok: true })
+  expect(nock.isDone()).toBeTruthy()
+})
+
+test('redirect without location header throws error', async () => {
+  nock('http://registry.pnpm.io/')
+    .get('/missing-location')
+    .reply(302, 'found')
+
+  const fetchFromRegistry = createFetchFromRegistry({})
+  await expect(fetchFromRegistry(
+    'http://registry.pnpm.io/missing-location'
+  )).rejects.toThrow(/Redirect location header missing/)
+})

network/fetching-types/src/index.ts

@@ -13,6 +13,7 @@ export type FetchFromRegistry = (
   opts?: RequestInit & {
     authHeaderValue?: string
     compress?: boolean
+    fullMetadata?: boolean
     retry?: RetryTimeoutOptions
     timeout?: number
   }

pkg-manager/package-requester/test/index.ts

@@ -1174,3 +1174,62 @@ test('HTTP tarball without integrity gets integrity computed during fetch', asyn
   expect(pkgResponse.body.resolution).toHaveProperty('integrity')
   expect((pkgResponse.body.resolution as { integrity?: string }).integrity).toMatch(/^sha512-/)
 })
+
+test('should pass optional flag to resolve function', async () => {
+  const storeDir = temporaryDirectory()
+  const cafs = createCafsStore(storeDir)
+
+  let capturedOptional: boolean | undefined
+  const mockResolve: typeof resolve = async (wantedDependency, _options) => {
+    capturedOptional = wantedDependency.optional
+    return resolve(wantedDependency, _options)
+  }
+
+  const requestPackage = createPackageRequester({
+    resolve: mockResolve,
+    fetchers,
+    cafs,
+    networkConcurrency: 1,
+    storeDir,
+    verifyStoreIntegrity: true,
+    virtualStoreDirMaxLength: 120,
+  })
+
+  const projectDir = temporaryDirectory()
+
+  await requestPackage(
+    { alias: 'is-positive', bareSpecifier: '1.0.0', optional: true },
+    {
+      downloadPriority: 0,
+      lockfileDir: projectDir,
+      preferredVersions: {},
+      projectDir,
+    }
+  )
+
+  expect(capturedOptional).toBe(true)
+
+  await requestPackage(
+    { alias: 'is-positive', bareSpecifier: '1.0.0', optional: false },
+    {
+      downloadPriority: 0,
+      lockfileDir: projectDir,
+      preferredVersions: {},
+      projectDir,
+    }
+  )
+
+  expect(capturedOptional).toBe(false)
+
+  await requestPackage(
+    { alias: 'is-positive', bareSpecifier: '1.0.0' },
+    {
+      downloadPriority: 0,
+      lockfileDir: projectDir,
+      preferredVersions: {},
+      projectDir,
+    }
+  )
+
+  expect(capturedOptional).toBeUndefined()
+})

pnpm/test/dlx.ts

@@ -351,12 +351,13 @@ describeOnLinuxOnly('dlx with supportedArchitectures CLI options', () => {
       '@pnpm.e2e/only-darwin-arm64',
       '@pnpm.e2e/only-darwin-x64',
       '@pnpm.e2e/only-linux-arm64-glibc',
-      '@pnpm.e2e/only-linux-arm64-musl',
       '@pnpm.e2e/only-linux-x64-glibc',
-      '@pnpm.e2e/only-linux-x64-musl',
       '@pnpm.e2e/only-win32-arm64',
       '@pnpm.e2e/only-win32-x64',
-    ], []],
+    ], [
+      '@pnpm.e2e/only-linux-arm64-musl',
+      '@pnpm.e2e/only-linux-x64-musl',
+    ]],
   ] as Case[])('%p', async (cliOpts, installed, notInstalled) => {
     prepareEmpty()
 

pnpm/test/install/supportedArchitectures.ts

@@ -28,12 +28,12 @@ type Case = [
 const TEST_CASES: Case[] = [
   [[], undefined, [
     'only-linux-x64-glibc',
-    'only-linux-x64-musl',
   ], [
     'only-darwin-arm64',
     'only-darwin-x64',
     'only-linux-arm64-glibc',
     'only-linux-arm64-musl',
+    'only-linux-x64-musl',
     'only-win32-arm64',
     'only-win32-x64',
   ]],

resolving/npm-resolver/src/fetch.ts

@@ -49,11 +49,20 @@ export interface FetchMetadataFromFromRegistryOptions {
   fetchWarnTimeoutMs: number
 }
 
+export interface FetchMetadataOptions {
+  registry: string
+  authHeaderValue?: string
+  fullMetadata?: boolean
+}
+
 export async function fetchMetadataFromFromRegistry (
   fetchOpts: FetchMetadataFromFromRegistryOptions,
   pkgName: string,
-  registry: string,
-  authHeaderValue?: string
+  {
+    authHeaderValue,
+    fullMetadata,
+    registry,
+  }: FetchMetadataOptions
 ): Promise<PackageMeta> {
   const uri = toUri(pkgName, registry)
   const op = retry.operation(fetchOpts.retry)
@@ -65,6 +74,7 @@ export async function fetchMetadataFromFromRegistry (
         response = await fetchOpts.fetch(uri, {
           authHeaderValue,
           compress: true,
+          fullMetadata,
           retry: fetchOpts.retry,
           timeout: fetchOpts.timeout,
         }) as RegistryResponse

resolving/npm-resolver/src/index.ts

@@ -1,5 +1,4 @@
 import path from 'path'
-import { FULL_META_DIR, FULL_FILTERED_META_DIR, ABBREVIATED_META_DIR } from '@pnpm/constants'
 import { PnpmError } from '@pnpm/error'
 import {
   type FetchFromRegistry,
@@ -149,7 +148,7 @@ export interface WorkspaceResolveResult extends ResolveResult {
 }
 
 export type NpmResolver = (
-  wantedDependency: WantedDependency,
+  wantedDependency: WantedDependency & { optional?: boolean },
   opts: ResolveFromNpmOptions
 ) => Promise<NpmResolveResult | JsrResolveResult | WorkspaceResolveResult | null>
 
@@ -179,9 +178,9 @@ export function createNpmResolver (
     getAuthHeaderValueByURI: getAuthHeader,
     pickPackage: pickPackage.bind(null, {
       fetch,
+      fullMetadata: opts.fullMetadata,
       filterMetadata: opts.filterMetadata,
       metaCache,
-      metaDir: opts.fullMetadata ? (opts.filterMetadata ? FULL_FILTERED_META_DIR : FULL_META_DIR) : ABBREVIATED_META_DIR,
       offline: opts.offline,
       preferOffline: opts.preferOffline,
       cacheDir: opts.cacheDir,
@@ -233,7 +232,7 @@ export type ResolveFromNpmOptions = {
 
 async function resolveNpm (
   ctx: ResolveFromNpmContext,
-  wantedDependency: WantedDependency,
+  wantedDependency: WantedDependency & { optional?: boolean },
   opts: ResolveFromNpmOptions
 ): Promise<NpmResolveResult | WorkspaceResolveResult | null> {
   const defaultTag = opts.defaultTag ?? 'latest'
@@ -276,6 +275,7 @@ async function resolveNpm (
       preferredVersionSelectors: opts.preferredVersions?.[spec.name],
       registry,
       updateToLatest: opts.update === 'latest',
+      optional: wantedDependency.optional,
     })
   } catch (err: any) { // eslint-disable-line
     if ((workspacePackages != null) && opts.projectDir) {

resolving/npm-resolver/src/pickPackage.ts

@@ -1,5 +1,6 @@
 import { promises as fs } from 'fs'
 import path from 'path'
+import { ABBREVIATED_META_DIR, FULL_META_DIR, FULL_FILTERED_META_DIR } from '@pnpm/constants'
 import { createHexHash } from '@pnpm/crypto.hash'
 import { PnpmError } from '@pnpm/error'
 import { logger } from '@pnpm/logger'
@@ -66,6 +67,7 @@ export interface PickPackageOptions extends PickPackageFromMetaOptions {
   registry: string
   dryRun: boolean
   updateToLatest?: boolean
+  optional?: boolean
 }
 
 const pickPackageFromMetaUsingTimeStrict = pickPackageFromMeta.bind(null, pickVersionByVersionRange)
@@ -84,8 +86,8 @@ function pickPackageFromMetaUsingTime (
 
 export async function pickPackage (
   ctx: {
-    fetch: (pkgName: string, registry: string, authHeaderValue?: string) => Promise<PackageMeta>
-    metaDir: string
+    fetch: (pkgName: string, opts: { registry: string, authHeaderValue?: string, fullMetadata?: boolean }) => Promise<PackageMeta>
+    fullMetadata?: boolean
     metaCache: PackageMetaCache
     cacheDir: string
     offline?: boolean
@@ -125,7 +127,15 @@ export async function pickPackage (
 
   validatePackageName(spec.name)
 
-  const cachedMeta = ctx.metaCache.get(spec.name)
+  // Use full metadata for optional dependencies to get libc field.
+  // See: https://github.com/pnpm/pnpm/issues/9950
+  const fullMetadata = opts.optional === true || ctx.fullMetadata === true
+  const metaDir = fullMetadata
+    ? (ctx.filterMetadata ? FULL_FILTERED_META_DIR : FULL_META_DIR)
+    : ABBREVIATED_META_DIR
+  // Cache key includes fullMetadata to avoid returning abbreviated metadata when full metadata is requested.
+  const cacheKey = fullMetadata ? `${spec.name}:full` : spec.name
+  const cachedMeta = ctx.metaCache.get(cacheKey)
   if (cachedMeta != null) {
     return {
       meta: cachedMeta,
@@ -134,7 +144,7 @@ export async function pickPackage (
   }
 
   const registryName = getRegistryName(opts.registry)
-  const pkgMirror = path.join(ctx.cacheDir, ctx.metaDir, registryName, `${encodePkgName(spec.name)}.json`)
+  const pkgMirror = path.join(ctx.cacheDir, metaDir, registryName, `${encodePkgName(spec.name)}.json`)
 
   return runLimited(pkgMirror, async (limit) => {
     let metaCachedInStore: PackageMeta | null | undefined
@@ -201,13 +211,17 @@ export async function pickPackage (
     }
 
     try {
-      let meta = await ctx.fetch(spec.name, opts.registry, opts.authHeaderValue)
+      let meta = await ctx.fetch(spec.name, {
+        authHeaderValue: opts.authHeaderValue,
+        fullMetadata,
+        registry: opts.registry,
+      })
       if (ctx.filterMetadata) {
         meta = clearMeta(meta)
       }
       meta.cachedAt = Date.now()
       // only save meta to cache, when it is fresh
-      ctx.metaCache.set(spec.name, meta)
+      ctx.metaCache.set(cacheKey, meta)
       if (!opts.dryRun) {
         // We stringify this meta here to avoid saving any mutations that could happen to the meta object.
         const stringifiedMeta = JSON.stringify(meta)

resolving/npm-resolver/test/optionalDependencies.test.ts

@@ -0,0 +1,146 @@
+/// <reference path="../../../__typings__/index.d.ts"/>
+import { createFetchFromRegistry } from '@pnpm/fetch'
+import { createNpmResolver } from '@pnpm/npm-resolver'
+import { type Registries } from '@pnpm/types'
+import nock from 'nock'
+import tempy from 'tempy'
+
+const registries = {
+  default: 'https://registry.npmjs.org/',
+} satisfies Registries
+
+const fetch = createFetchFromRegistry({})
+const getAuthHeader = () => undefined
+const createResolveFromNpm = createNpmResolver.bind(null, fetch, getAuthHeader)
+
+afterEach(() => {
+  nock.cleanAll()
+  nock.disableNetConnect()
+})
+
+beforeEach(() => {
+  nock.enableNetConnect()
+})
+
+describe('optional dependencies', () => {
+  test('optional dependencies receive full metadata with libc field', async () => {
+    // This test verifies the fix for https://github.com/pnpm/pnpm/issues/9950
+    // Optional dependencies need full metadata to get the libc field for platform compatibility checks.
+    const packageMeta = {
+      name: 'platform-pkg',
+      'dist-tags': { latest: '1.0.0' },
+      versions: {
+        '1.0.0': {
+          name: 'platform-pkg',
+          version: '1.0.0',
+          os: ['linux'],
+          cpu: ['x64'],
+          libc: ['glibc'],
+          dist: {
+            tarball: 'https://registry.npmjs.org/platform-pkg/-/platform-pkg-1.0.0.tgz',
+            integrity: 'sha512-test1234567890123456789012345678901234567890123456789012345678',
+          },
+        },
+      },
+    }
+
+    // Verify that full metadata is requested (no abbreviated Accept header)
+    const scope = nock(registries.default)
+      .get('/platform-pkg')
+      .matchHeader('accept', (value) => !value.includes('application/vnd.npm.install-v1+json'))
+      .reply(200, packageMeta)
+
+    const { resolveFromNpm } = createResolveFromNpm({
+      cacheDir: tempy.directory(),
+      registries,
+    })
+
+    const result = await resolveFromNpm(
+      {
+        alias: 'platform-pkg',
+        bareSpecifier: '1.0.0',
+        optional: true,
+      },
+      {}
+    )
+
+    expect(result!.manifest!.libc).toEqual(['glibc'])
+    expect(result!.manifest!.os).toEqual(['linux'])
+    expect(result!.manifest!.cpu).toEqual(['x64'])
+    expect(scope.isDone()).toBe(true)
+  })
+
+  test('abbreviated and full metadata are cached separately', async () => {
+    // Abbreviated metadata doesn't include scripts, full metadata does.
+    // When resolving the same package first as regular, then as optional,
+    // we should get different metadata from each request.
+    const abbreviatedMeta = {
+      name: 'cache-test',
+      'dist-tags': { latest: '1.0.0' },
+      versions: {
+        '1.0.0': {
+          name: 'cache-test',
+          version: '1.0.0',
+          dist: {
+            tarball: 'https://registry.npmjs.org/cache-test/-/cache-test-1.0.0.tgz',
+            integrity: 'sha512-test1234567890123456789012345678901234567890123456789012345678',
+          },
+        },
+      },
+    }
+    const fullMeta = {
+      name: 'cache-test',
+      'dist-tags': { latest: '1.0.0' },
+      versions: {
+        '1.0.0': {
+          name: 'cache-test',
+          version: '1.0.0',
+          scripts: {
+            test: 'jest',
+            build: 'tsc',
+          },
+          dist: {
+            tarball: 'https://registry.npmjs.org/cache-test/-/cache-test-1.0.0.tgz',
+            integrity: 'sha512-test1234567890123456789012345678901234567890123456789012345678',
+          },
+        },
+      },
+    }
+
+    // First request: abbreviated metadata for regular dependency
+    const abbreviatedScope = nock(registries.default)
+      .get('/cache-test')
+      .matchHeader('accept', /application\/vnd\.npm\.install-v1\+json/)
+      .reply(200, abbreviatedMeta)
+
+    // Second request: full metadata for optional dependency
+    const fullScope = nock(registries.default)
+      .get('/cache-test')
+      .matchHeader('accept', (value) => !value.includes('application/vnd.npm.install-v1+json'))
+      .reply(200, fullMeta)
+
+    const cacheDir = tempy.directory()
+
+    const { resolveFromNpm } = createResolveFromNpm({
+      cacheDir,
+      registries,
+    })
+
+    // Resolve as regular dependency - should get abbreviated metadata
+    const regularResult = await resolveFromNpm(
+      { alias: 'cache-test', bareSpecifier: '1.0.0' },
+      {}
+    )
+    expect(regularResult!.manifest!.scripts).toBeUndefined()
+
+    // Resolve as optional dependency - should get full metadata (separate cache entry)
+    const optionalResult = await resolveFromNpm(
+      { alias: 'cache-test', bareSpecifier: '1.0.0', optional: true },
+      {}
+    )
+    expect(optionalResult!.manifest!.scripts).toEqual({ test: 'jest', build: 'tsc' })
+
+    expect(abbreviatedScope.isDone()).toBe(true)
+    expect(fullScope.isDone()).toBe(true)
+  })
+})

resolving/resolver-base/src/index.ts

@@ -137,4 +137,4 @@ export type WantedDependency = {
   bareSpecifier?: string
 })
 
-export type ResolveFunction = (wantedDependency: WantedDependency, opts: ResolveOptions) => Promise<ResolveResult>
+export type ResolveFunction = (wantedDependency: WantedDependency & { optional?: boolean }, opts: ResolveOptions) => Promise<ResolveResult>