diff --git a/.changeset/honest-steaks-cheat.md b/.changeset/honest-steaks-cheat.md new file mode 100644 index 0000000000..a8aecd41fa --- /dev/null +++ b/.changeset/honest-steaks-cheat.md @@ -0,0 +1,5 @@ +--- +"@pnpm/resolve-dependencies": patch +--- + +Only add packages to the skipped set, when they are seen the first time. diff --git a/.changeset/serious-plants-fix.md b/.changeset/serious-plants-fix.md new file mode 100644 index 0000000000..a868941097 --- /dev/null +++ b/.changeset/serious-plants-fix.md @@ -0,0 +1,5 @@ +--- +"@pnpm/prune-lockfile": patch +--- + +Don't remove optional dependencies of optional dependencies. diff --git a/packages/prune-lockfile/src/index.ts b/packages/prune-lockfile/src/index.ts index feb269d4b5..0d83e6ad76 100644 --- a/packages/prune-lockfile/src/index.ts +++ b/packages/prune-lockfile/src/index.ts @@ -18,9 +18,9 @@ export function pruneSharedLockfile ( } ) { const copiedPackages = !lockfile.packages ? {} : copyPackageSnapshots(lockfile.packages, { - devRelPaths: R.unnest(R.values(lockfile.importers).map((deps) => resolvedDepsToRelDepPaths(deps.devDependencies || {}))), - optionalRelPaths: R.unnest(R.values(lockfile.importers).map((deps) => resolvedDepsToRelDepPaths(deps.optionalDependencies || {}))), - prodRelPaths: R.unnest(R.values(lockfile.importers).map((deps) => resolvedDepsToRelDepPaths(deps.dependencies || {}))), + devDepPaths: R.unnest(R.values(lockfile.importers).map((deps) => resolvedDepsToDepPaths(deps.devDependencies ?? {}))), + optionalDepPaths: R.unnest(R.values(lockfile.importers).map((deps) => resolvedDepsToDepPaths(deps.optionalDependencies ?? {}))), + prodDepPaths: R.unnest(R.values(lockfile.importers).map((deps) => resolvedDepsToDepPaths(deps.dependencies ?? {}))), warn: opts && opts.warn || ((msg: string) => undefined), }) @@ -44,7 +44,7 @@ export function pruneLockfile ( ): Lockfile { const packages: PackageSnapshots = {} const importer = lockfile.importers[importerId] - const lockfileSpecs: ResolvedDependencies = importer.specifiers || {} + const lockfileSpecs: ResolvedDependencies = importer.specifiers ?? {} const optionalDependencies = R.keys(pkg.optionalDependencies) const dependencies = R.difference(R.keys(pkg.dependencies), optionalDependencies) const devDependencies = R.difference(R.difference(R.keys(pkg.devDependencies), optionalDependencies), dependencies) @@ -111,106 +111,90 @@ export function pruneLockfile ( function copyPackageSnapshots ( originalPackages: PackageSnapshots, opts: { - devRelPaths: string[], - optionalRelPaths: string[], - prodRelPaths: string[], + devDepPaths: string[], + optionalDepPaths: string[], + prodDepPaths: string[], warn: (msg: string) => void, } ): PackageSnapshots { - const copiedPackages: PackageSnapshots = {} - const nonOptional = new Set() - const notProdOnly = new Set() + const copiedSnapshots: PackageSnapshots = {} + const ctx = { + copiedSnapshots, + nonOptional: new Set(), + notProdOnly: new Set(), + originalPackages, + walked: new Set(), + warn: opts.warn, + } - copyDependencySubGraph(copiedPackages, opts.devRelPaths, originalPackages, new Set(), opts.warn, { + copyDependencySubGraph(ctx, opts.devDepPaths, { dev: true, - nonOptional, - notProdOnly, + optional: false, }) - - copyDependencySubGraph(copiedPackages, opts.prodRelPaths, originalPackages, new Set(), opts.warn, { - nonOptional, - notProdOnly, - }) - - copyDependencySubGraph(copiedPackages, opts.optionalRelPaths, originalPackages, new Set(), opts.warn, { - nonOptional, - notProdOnly, + copyDependencySubGraph(ctx, opts.optionalDepPaths, { + dev: false, optional: true, }) - - copyDependencySubGraph(copiedPackages, opts.devRelPaths, originalPackages, new Set(), opts.warn, { - dev: true, - nonOptional, - notProdOnly, - walkOptionals: true, + copyDependencySubGraph(ctx, opts.prodDepPaths, { + dev: false, + optional: false, }) - copyDependencySubGraph(copiedPackages, opts.prodRelPaths, originalPackages, new Set(), opts.warn, { - nonOptional, - notProdOnly, - walkOptionals: true, - }) - - return copiedPackages + return copiedSnapshots } -function resolvedDepsToRelDepPaths (deps: ResolvedDependencies) { - return Object.keys(deps) - .map((pkgName) => refToRelative(deps[pkgName], pkgName)) - .filter((relPath) => relPath !== null) as string[] +function resolvedDepsToDepPaths (deps: ResolvedDependencies) { + return Object.entries(deps) + .map(([alias, ref]) => refToRelative(ref, alias)) + .filter((depPath) => depPath !== null) as string[] } function copyDependencySubGraph ( - copiedSnapshots: PackageSnapshots, - depRelativePaths: string[], - originalPackages: PackageSnapshots, - walked: Set, - warn: (msg: string) => void, - opts: { - dev?: boolean, - optional?: boolean, + ctx: { + copiedSnapshots: PackageSnapshots, nonOptional: Set, notProdOnly: Set, - walkOptionals?: boolean, + originalPackages: PackageSnapshots, + walked: Set, + warn: (msg: string) => void, + }, + depPaths: string[], + opts: { + dev: boolean, + optional: boolean, } ) { - for (const depRalativePath of depRelativePaths) { - if (walked.has(depRalativePath)) continue - walked.add(depRalativePath) - if (!originalPackages[depRalativePath]) { + for (const depPath of depPaths) { + const key = `${depPath}:${opts.optional}:${opts.dev}` + if (ctx.walked.has(key)) continue + ctx.walked.add(key) + if (!ctx.originalPackages[depPath]) { // local dependencies don't need to be resolved in pnpm-lock.yaml // except local tarball dependencies - if (depRalativePath.startsWith('link:') || depRalativePath.startsWith('file:') && !depRalativePath.endsWith('.tar.gz')) continue + if (depPath.startsWith('link:') || depPath.startsWith('file:') && !depPath.endsWith('.tar.gz')) continue - // NOTE: Warnings should not be printed for the current lockfile (node_modules/.lockfile.yaml). - // The current lockfile does not contain the skipped packages, so it may have missing resolutions - warn(`Cannot find resolution of ${depRalativePath} in lockfile`) + ctx.warn(`Cannot find resolution of ${depPath} in lockfile`) continue } - const depLockfile = originalPackages[depRalativePath] - copiedSnapshots[depRalativePath] = depLockfile - if (opts.optional && !opts.nonOptional.has(depRalativePath)) { + const depLockfile = ctx.originalPackages[depPath] + ctx.copiedSnapshots[depPath] = depLockfile + if (opts.optional && !ctx.nonOptional.has(depPath)) { depLockfile.optional = true } else { - opts.nonOptional.add(depRalativePath) + ctx.nonOptional.add(depPath) delete depLockfile.optional } if (opts.dev) { - opts.notProdOnly.add(depRalativePath) + ctx.notProdOnly.add(depPath) depLockfile.dev = true } else if (depLockfile.dev === true) { // keeping if dev is explicitly false delete depLockfile.dev - } else if (depLockfile.dev === undefined && !opts.notProdOnly.has(depRalativePath)) { + } else if (depLockfile.dev === undefined && !ctx.notProdOnly.has(depPath)) { depLockfile.dev = false } - const newDependencies = R.keys(depLockfile.dependencies) - .map((pkgName: string) => refToRelative((depLockfile.dependencies && depLockfile.dependencies[pkgName]) as string, pkgName)) - .filter((relPath) => relPath !== null) as string[] - copyDependencySubGraph(copiedSnapshots, newDependencies, originalPackages, walked, warn, opts) - if (!opts.walkOptionals) continue - const newOptionalDependencies = R.keys(depLockfile.optionalDependencies) - .map((pkgName: string) => refToRelative((depLockfile.optionalDependencies && depLockfile.optionalDependencies[pkgName]) as string, pkgName)) - .filter((relPath) => relPath !== null) as string[] - copyDependencySubGraph(copiedSnapshots, newOptionalDependencies, originalPackages, walked, warn, { ...opts, optional: true }) + const newDependencies = resolvedDepsToDepPaths(depLockfile.dependencies ?? {}) + copyDependencySubGraph(ctx, newDependencies, opts) + const newOptionalDependencies = resolvedDepsToDepPaths(depLockfile.optionalDependencies ?? {}) + copyDependencySubGraph(ctx, newOptionalDependencies, { dev: opts.dev, optional: true }) } } diff --git a/packages/resolve-dependencies/src/resolveDependencies.ts b/packages/resolve-dependencies/src/resolveDependencies.ts index 303ea6dd0c..fb11dca7b4 100644 --- a/packages/resolve-dependencies/src/resolveDependencies.ts +++ b/packages/resolve-dependencies/src/resolveDependencies.ts @@ -679,13 +679,13 @@ async function resolveDependency ( pnpmVersion: ctx.pnpmVersion, }) ) - if (currentIsInstallable !== true || !parentIsInstallable) { - ctx.skipped.add(pkgResponse.body.id) - } const installable = parentIsInstallable && currentIsInstallable !== false const isNew = !ctx.resolvedPackagesByPackageId[pkgResponse.body.id] if (isNew) { + if (currentIsInstallable !== true || !parentIsInstallable) { + ctx.skipped.add(pkgResponse.body.id) + } progressLogger.debug({ packageId: pkgResponse.body.id, requester: ctx.lockfileDir, diff --git a/packages/supi/test/install/optionalDependencies.ts b/packages/supi/test/install/optionalDependencies.ts index 8028a8c345..aa74672e94 100644 --- a/packages/supi/test/install/optionalDependencies.ts +++ b/packages/supi/test/install/optionalDependencies.ts @@ -341,8 +341,24 @@ test('optional subdependency is skipped', async (t: tape.Test) => { } }) +// Covers https://github.com/pnpm/pnpm/issues/2663 +test('optional subdependency of newly added optional dependency is skipped', async (t: tape.Test) => { + const project = prepareEmpty(t) + const reporter = sinon.spy() + + const manifest = await addDependenciesToPackage({}, ['pkg-with-optional'], await testDefaults({ reporter, targetDependenciesField: 'optionalDependencies' })) + + const modulesInfo = await readYamlFile<{ skipped: string[] }>(path.join('node_modules', '.modules.yaml')) + t.deepEqual(modulesInfo.skipped, ['/dep-of-optional-pkg/1.0.0', '/not-compatible-with-any-os/1.0.0'], 'optional subdep skipped') + + const lockfile = await project.readLockfile() + + t.equal(Object.keys(lockfile.packages).length, 3) + t.ok(lockfile.packages['/not-compatible-with-any-os/1.0.0']) +}) + test('only that package is skipped which is an optional dependency only and not installable', async (t) => { - prepareEmpty(t) + const project = prepareEmpty(t) const reporter = sinon.spy() const manifest = await addDependenciesToPackage({}, [ @@ -356,6 +372,9 @@ test('only that package is skipped which is an optional dependency only and not t.deepEqual(modulesInfo.skipped, ['/not-compatible-with-any-os-and-has-peer/1.0.0_peer-c@1.0.0']) } + const lockfile = await project.readLockfile() + t.equal(typeof lockfile.packages['/dep-of-optional-pkg/1.0.0'].optional, 'undefined') + await rimraf('node_modules') await mutateModules(