diff --git a/src/parsers/index.ts b/src/parsers/index.ts index def416df8..7224d33b9 100644 --- a/src/parsers/index.ts +++ b/src/parsers/index.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import path from "node:path"; -import { LOCKFILE_NAMES, LOCKFILE_PRIORITY_ROOT, type LockfileName } from "../constants.js"; +import { LOCKFILE_NAMES, LOCKFILE_PRIORITY_ROOT, NPM_LOCKFILE_NAMES, type LockfileName } from "../constants.js"; import type { PackageRef, ScanInput, ScanSource } from "../types.js"; import { findFiles, findNearestPackageJson, chooseBestLockfile, safeReadText, relativeOrName } from "../utils/file.js"; import { loadFromBunLock } from "./bun-lock.js"; @@ -72,12 +72,12 @@ interface FoundLockfile { readonly declaredBy?: string; } -/** The lockfile each package manager writes, for reading `packageManager`. */ -const PACKAGE_MANAGER_LOCKFILES: Readonly> = { - npm: "package-lock.json", - pnpm: "pnpm-lock.yaml", - yarn: "yarn.lock", - bun: "bun.lock", +/** Each package manager's lockfiles, in the order it prefers them. */ +const PACKAGE_MANAGER_LOCKFILES: Readonly> = { + npm: NPM_LOCKFILE_NAMES, + pnpm: ["pnpm-lock.yaml"], + yarn: ["yarn.lock"], + bun: ["bun.lock"], }; /** @@ -92,7 +92,7 @@ function readDeclaredPackageManager(projectRoot: string): string | null { const declared = (JSON.parse(manifest) as { packageManager?: unknown }).packageManager; if (typeof declared !== "string") return null; const name = declared.split("@")[0]?.trim().toLowerCase(); - return name && name in PACKAGE_MANAGER_LOCKFILES ? name : null; + return name && Object.hasOwn(PACKAGE_MANAGER_LOCKFILES, name) ? name : null; } catch { // A manifest we cannot parse tells us nothing about the package manager. // Lockfile selection falls back to precedence rather than failing here. @@ -116,15 +116,10 @@ function findRootLockfile(projectRoot: string): FoundLockfile | null { if (present.length === 0) return null; const declared = readDeclaredPackageManager(projectRoot); - // npm treats npm-shrinkwrap.json as authoritative over package-lock.json. - // Keep that rule even when package.json declares npm, while preserving the - // packageManager override for other ecosystems (for example, pnpm beside a - // stale package-lock.json). - const declaredLockfile = declared === "npm" && present.includes("npm-shrinkwrap.json") - ? "npm-shrinkwrap.json" - : declared ? PACKAGE_MANAGER_LOCKFILES[declared] : undefined; - const chosen = - declaredLockfile && present.includes(declaredLockfile) ? declaredLockfile : present[0]!; + const declaredLockfile = declared + ? PACKAGE_MANAGER_LOCKFILES[declared]?.find(name => present.includes(name)) + : undefined; + const chosen = declaredLockfile ?? present[0]!; return { filePath: path.join(projectRoot, chosen), @@ -138,6 +133,9 @@ function findRootLockfile(projectRoot: string): FoundLockfile | null { function describeLockfileChoice(found: FoundLockfile): string[] { if (found.skipped.length === 0) return []; const others = found.skipped.join(", "); + if (found.name === "npm-shrinkwrap.json" && found.skipped.length === 1 && found.skipped[0] === "package-lock.json") { + return [`Root also contains ${others}. Used npm-shrinkwrap.json because npm reads it ahead of package-lock.json.`]; + } return found.declaredBy ? [`Root also contains ${others}. Used ${found.name} because package.json declares packageManager "${found.declaredBy}".`] : [`Root also contains ${others}. Used ${found.name} by precedence; no packageManager field declared.`]; diff --git a/tests/parsers/load-packages.test.ts b/tests/parsers/load-packages.test.ts index d2eeb22e6..73b86acd1 100644 --- a/tests/parsers/load-packages.test.ts +++ b/tests/parsers/load-packages.test.ts @@ -195,6 +195,9 @@ packages: expect.arrayContaining([expect.objectContaining({ name: "lodash" })]), ); expect(result.packages.map(p => p.name)).not.toContain("express"); + expect(result.notes).toContain( + "Root also contains package-lock.json. Used npm-shrinkwrap.json because npm reads it ahead of package-lock.json.", + ); } finally { removeDir(projectDir); } @@ -238,7 +241,9 @@ packages: expect(path.basename(result.filePath ?? "")).toBe("npm-shrinkwrap.json"); expect(result.packages.map(p => p.name)).toContain("shrinkwrap-package"); expect(result.packages.map(p => p.name)).not.toContain("stale-package"); - expect(result.notes.find(note => note.includes("packageManager \"npm\""))).toBeDefined(); + expect(result.notes).toContain( + "Root also contains package-lock.json. Used npm-shrinkwrap.json because npm reads it ahead of package-lock.json.", + ); } finally { removeDir(projectDir); } @@ -337,6 +342,40 @@ snapshots: fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify(manifest), "utf8"); } + it.each([ + { + label: "filename precedence with pnpm also present", + manifest: { name: "app" }, + includeBun: false, + expectedNote: "Root also contains package-lock.json, pnpm-lock.yaml. Used npm-shrinkwrap.json by precedence; no packageManager field declared.", + }, + { + label: "the npm declaration with Bun and pnpm also present", + manifest: { name: "app", packageManager: "npm@11.0.0" }, + includeBun: true, + expectedNote: 'Root also contains bun.lock, package-lock.json, pnpm-lock.yaml. Used npm-shrinkwrap.json because package.json declares packageManager "npm".', + }, + ])("explains $label when shrinkwrap wins", ({ manifest, includeBun, expectedNote }) => { + const projectDir = createTempProjectDir(); + try { + writeBothLockfiles(projectDir, manifest); + fs.writeFileSync(path.join(projectDir, "npm-shrinkwrap.json"), JSON.stringify({ + lockfileVersion: 3, + packages: { "node_modules/shrinkwrap-package": { version: "1.0.0" } }, + }), "utf8"); + if (includeBun) { + fs.writeFileSync(path.join(projectDir, "bun.lock"), "{}", "utf8"); + } + + const result = loadPackages(projectDir, false, 4); + expect(result.source).toBe("npm-shrinkwrap"); + expect(result.packages.map(p => p.name)).toEqual(["shrinkwrap-package"]); + expect(result.notes).toContain(expectedNote); + } finally { + removeDir(projectDir); + } + }); + it("uses the lockfile the project declares in packageManager, not filename precedence", () => { const projectDir = createTempProjectDir(); // Exactly the caveman shape: a pnpm workspace with a stale package-lock.json @@ -372,6 +411,23 @@ snapshots: } }); + it("uses package-lock.json when npm is declared and no shrinkwrap is present", () => { + const projectDir = createTempProjectDir(); + writeBothLockfiles(projectDir, { name: "app", packageManager: "npm@11.0.0" }); + + try { + const result = loadPackages(projectDir, false, 4); + expect(result.source).toBe("package-lock"); + expect(result.packages.map(p => p.name)).toContain("leftpad"); + expect(result.packages.map(p => p.name)).not.toContain("chalk"); + expect(result.notes).toContain( + 'Root also contains pnpm-lock.yaml. Used package-lock.json because package.json declares packageManager "npm".', + ); + } finally { + removeDir(projectDir); + } + }); + it("falls back to precedence when no packageManager is declared, and says so", () => { const projectDir = createTempProjectDir(); writeBothLockfiles(projectDir, { name: "app" }); @@ -386,10 +442,10 @@ snapshots: } }); - it("ignores a packageManager naming a lockfile that is not present", () => { + it.each(["yarn@4.0.0", "constructor", "__proto__"])("falls back to precedence when packageManager %s has no matching lockfile", (packageManager) => { const projectDir = createTempProjectDir(); - // Declares yarn, but only the pnpm and npm lockfiles exist. - writeBothLockfiles(projectDir, { name: "app", packageManager: "yarn@4.0.0" }); + // Only pnpm and npm lockfiles exist; unsupported names must also fall back safely. + writeBothLockfiles(projectDir, { name: "app", packageManager }); try { const result = loadPackages(projectDir, false, 4);