fix: dependencies that are not only optional should be installed when optionals are skipped (#8076)

close #8066

Co-authored-by: khai96_ <hvksmr1996@gmail.com>

.changeset/stupid-buckets-think.md

@@ -0,0 +1,8 @@
+---
+"@pnpm/prune-lockfile": minor
+"@pnpm/resolve-dependencies": patch
+"@pnpm/core": patch
+"pnpm": patch
+---
+
+Fix a bug in which a dependency that is both optional for one package but non-optional for another is omitted when `optional=false` [#8066](https://github.com/pnpm/pnpm/issues/8066).

lockfile/prune-lockfile/src/index.ts

@@ -13,9 +13,13 @@ import unnest from 'ramda/src/unnest'
 
 export * from '@pnpm/lockfile-types'
 
+// cannot import DependenciesGraph from @pnpm/resolve-dependencies due to circular dependency
+type DependenciesGraph = Record<DepPath, { optional?: boolean }>
+
 export function pruneSharedLockfile (
   lockfile: Lockfile,
   opts?: {
+    dependenciesGraph?: DependenciesGraph
     warn?: (msg: string) => void
   }
 ): Lockfile {
@@ -26,6 +30,7 @@ export function pruneSharedLockfile (
       optionalDepPaths: unnest(Object.values(lockfile.importers).map((deps) => resolvedDepsToDepPaths(deps.optionalDependencies ?? {}))),
       prodDepPaths: unnest(Object.values(lockfile.importers).map((deps) => resolvedDepsToDepPaths(deps.dependencies ?? {}))),
       warn: opts?.warn ?? ((msg: string) => undefined),
+      dependenciesGraph: opts?.dependenciesGraph,
     })
 
   const prunedLockfile: Lockfile = {
@@ -42,8 +47,9 @@ export function pruneLockfile (
   lockfile: Lockfile,
   pkg: PackageManifest,
   importerId: string,
-  opts?: {
+  opts: {
     warn?: (msg: string) => void
+    dependenciesGraph?: DependenciesGraph
   }
 ): Lockfile {
   const importer = lockfile.importers[importerId]
@@ -121,6 +127,7 @@ function copyPackageSnapshots (
     optionalDepPaths: DepPath[]
     prodDepPaths: DepPath[]
     warn: (msg: string) => void
+    dependenciesGraph?: DependenciesGraph
   }
 ): PackageSnapshots {
   const copiedSnapshots: PackageSnapshots = {}
@@ -130,6 +137,7 @@ function copyPackageSnapshots (
     originalPackages,
     walked: new Set<string>(),
     warn: opts.warn,
+    dependenciesGraph: opts.dependenciesGraph,
   }
 
   copyDependencySubGraph(ctx, opts.devDepPaths, {
@@ -158,6 +166,7 @@ function copyDependencySubGraph (
     originalPackages: PackageSnapshots
     walked: Set<string>
     warn: (msg: string) => void
+    dependenciesGraph?: DependenciesGraph
   },
   depPaths: DepPath[],
   opts: {
@@ -180,9 +189,15 @@ function copyDependencySubGraph (
     ctx.copiedSnapshots[depPath] = depLockfile
     if (opts.optional && !ctx.nonOptional.has(depPath)) {
       depLockfile.optional = true
+      if (ctx.dependenciesGraph?.[depPath]) {
+        ctx.dependenciesGraph[depPath].optional = true
+      }
     } else {
       ctx.nonOptional.add(depPath)
       delete depLockfile.optional
+      if (ctx.dependenciesGraph?.[depPath]) {
+        ctx.dependenciesGraph[depPath].optional = false
+      }
     }
     const newDependencies = resolvedDepsToDepPaths(depLockfile.dependencies ?? {})
     copyDependencySubGraph(ctx, newDependencies, opts)

pkg-manager/core/test/install/optionalDependencies.ts

@@ -698,3 +698,18 @@ test('complex scenario with same optional dependencies appearing in many places
   expect(fs.readdirSync('node_modules/.pnpm/esbuild@0.18.20/node_modules/@esbuild').length).toEqual(1)
   expect(fs.readdirSync('node_modules/.pnpm/esbuild@0.20.2/node_modules/@esbuild').length).toEqual(1)
 })
+
+// Covers https://github.com/pnpm/pnpm/issues/8066
+test('dependency that is both optional and non-optional is installed, when optional dependencies should be skipped', async () => {
+  prepareEmpty()
+  await addDependenciesToPackage({}, ['@babel/cli@7.24.5', 'del@6.1.1'], testDefaults({
+    include: {
+      dependencies: true,
+      optionalDependencies: false,
+      devDependencies: true,
+    },
+  }))
+
+  const dirs = fs.readdirSync('node_modules/.pnpm')
+  expect(dirs.find(dir => dir.startsWith('fill-range@'))).toBeDefined()
+})

pkg-manager/resolve-dependencies/src/updateLockfile.ts

@@ -44,7 +44,7 @@ export function updateLockfile (
   const warn = (message: string) => {
     logger.warn({ message, prefix })
   }
-  return pruneSharedLockfile(lockfile, { warn })
+  return pruneSharedLockfile(lockfile, { warn, dependenciesGraph })
 }
 
 function toLockfileDependency (