From 2239a064722733e84c791192807f0e8f6baac57f Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Wed, 23 Sep 2026 17:05:13 +0200 Subject: [PATCH 1/2] feat: make plugin create output package-manager-aware via detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `appkit plugin create` previously hardcoded pnpm in its guidance. Detect the developer's package manager and render the right commands instead — with no CLI flag, since the command runs inside a project whose manager is already decided. - New shared module `cli/package-manager.ts`: `detectPackageManager` (order: `npm_config_user_agent` -> lockfile in cwd, including `package-lock.json` -> npm -> pnpm fallback) and a `PM_COMMANDS` capability map (install/build/add/exec) that always uses an explicit `run` form, e.g. `pnpm run build` (pnpm 11 shadows bare script names). - `printNextSteps` (isolated-placement branch) and the generated plugin README now render install/build/add for the detected manager. - `registry add` consolidated onto the shared detector (its local lockfile-only copy removed) for a single source of truth. Covers pnpm, npm, yarn, and bun. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- .../cli/commands/plugin/create/create.test.ts | 88 +++++++- .../src/cli/commands/plugin/create/create.ts | 45 +++- .../commands/plugin/create/scaffold.test.ts | 82 +++++++ .../cli/commands/plugin/create/scaffold.ts | 8 +- .../shared/src/cli/commands/registry/add.ts | 8 +- .../shared/src/cli/package-manager.test.ts | 210 ++++++++++++++++++ packages/shared/src/cli/package-manager.ts | 78 +++++++ 7 files changed, 499 insertions(+), 20 deletions(-) create mode 100644 packages/shared/src/cli/package-manager.test.ts create mode 100644 packages/shared/src/cli/package-manager.ts diff --git a/packages/shared/src/cli/commands/plugin/create/create.test.ts b/packages/shared/src/cli/commands/plugin/create/create.test.ts index fb47994fc..3b64a12ad 100644 --- a/packages/shared/src/cli/commands/plugin/create/create.test.ts +++ b/packages/shared/src/cli/commands/plugin/create/create.test.ts @@ -1,10 +1,12 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { buildResourceFromType, parseResourcesJson, parseResourcesShorthand, + printNextSteps, } from "./create"; +import type { CreateAnswers } from "./types"; describe("create non-interactive helpers", () => { describe("buildResourceFromType", () => { @@ -150,4 +152,88 @@ describe("create non-interactive helpers", () => { } }); }); + + describe("printNextSteps", () => { + let consoleLogSpy: ReturnType; + + beforeEach(() => { + consoleLogSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + const answers: CreateAnswers = { + placement: "isolated", + targetPath: "plugins/my-plugin", + name: "myPlugin", + displayName: "My Plugin", + description: "Test plugin", + resources: [], + version: "0.1.0", + }; + + const targetDir = "/home/user/project/plugins/my-plugin"; + + it("shows pnpm commands when pm is pnpm", () => { + printNextSteps(answers, targetDir, "pnpm"); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("pnpm install"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("pnpm run build"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("pnpm add"), + ); + }); + + it("shows npm commands when pm is npm", () => { + printNextSteps(answers, targetDir, "npm"); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("npm install"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("npm run build"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("npm install"), + ); + }); + + it("shows yarn commands when pm is yarn", () => { + printNextSteps(answers, targetDir, "yarn"); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("yarn install"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("yarn run build"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("yarn add"), + ); + }); + + it("shows bun commands when pm is bun", () => { + printNextSteps(answers, targetDir, "bun"); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("bun install"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("bun run build"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("bun add"), + ); + }); + + it("shows npx for in-repo placement (PM-neutral)", () => { + const inRepoAnswers = { ...answers, placement: "in-repo" as const }; + printNextSteps(inRepoAnswers, targetDir, "npm"); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("npx appkit plugin sync"), + ); + }); + }); }); diff --git a/packages/shared/src/cli/commands/plugin/create/create.ts b/packages/shared/src/cli/commands/plugin/create/create.ts index 7fb013533..ad9e6438a 100644 --- a/packages/shared/src/cli/commands/plugin/create/create.ts +++ b/packages/shared/src/cli/commands/plugin/create/create.ts @@ -5,6 +5,11 @@ import process from "node:process"; import { Command, Option } from "commander"; import { PLUGIN_NAME_PATTERN } from "../../../../naming"; +import { + detectPackageManager, + PM_COMMANDS, + type PackageManager, +} from "../../../package-manager"; import { promptOneResource } from "./prompt-resource"; import { DEFAULT_PERMISSION_BY_TYPE, @@ -116,7 +121,11 @@ function parseResourcesShorthand(csv: string): SelectedResource[] { return types.map(buildResourceFromType); } -function printNextSteps(answers: CreateAnswers, targetDir: string): void { +function printNextSteps( + answers: CreateAnswers, + targetDir: string, + pm: PackageManager, +): void { const relativePath = path.relative(process.cwd(), targetDir); const importPath = relativePath.startsWith(".") ? relativePath @@ -133,12 +142,16 @@ function printNextSteps(answers: CreateAnswers, targetDir: string): void { ` 2. Run \`npx appkit plugin sync --write\` to update appkit.plugins.json.\n`, ); } else { - console.log(` 1. cd into the new package and install dependencies:`); - console.log(` cd ${answers.targetPath} && pnpm install`); - console.log(` 2. Build: pnpm build`); - console.log( - ` 3. In your app: pnpm add ./${answers.targetPath} @databricks/appkit`, + const installCmd = PM_COMMANDS[pm].install; + const buildCmd = PM_COMMANDS[pm].build; + const addCmd = PM_COMMANDS[pm].add( + `./${answers.targetPath} @databricks/appkit`, ); + + console.log(` 1. cd into the new package and install dependencies:`); + console.log(` cd ${answers.targetPath} && ${installCmd}`); + console.log(` 2. Build: ${buildCmd}`); + console.log(` 3. In your app: ${addCmd}`); console.log( ` 4. Import and register: import { ${exportName} } from "";\n`, ); @@ -213,12 +226,16 @@ function runNonInteractive(opts: CreateOptions): void { process.exit(1); } - scaffoldPlugin(targetDir, answers, { isolated: placement === "isolated" }); + const pm = detectPackageManager(process.cwd()); + scaffoldPlugin(targetDir, answers, { + isolated: placement === "isolated", + pm, + }); console.log( `Plugin "${answers.name}" created at ${path.relative(process.cwd(), targetDir)}`, ); - printNextSteps(answers, targetDir); + printNextSteps(answers, targetDir, pm); } async function runInteractive(): Promise { @@ -403,8 +420,10 @@ async function runInteractive(): Promise { const s = spinner(); s.start("Writing files…"); try { + const pm = detectPackageManager(process.cwd()); scaffoldPlugin(targetDir, answers, { isolated: placement === "isolated", + pm, }); s.stop("Files written."); } catch (err) { @@ -413,7 +432,8 @@ async function runInteractive(): Promise { } outro("Plugin created successfully."); - printNextSteps(answers, targetDir); + const pm = detectPackageManager(process.cwd()); + printNextSteps(answers, targetDir, pm); } catch (err) { console.error(err); process.exit(1); @@ -485,4 +505,9 @@ Examples: ); /** Exported for testing. */ -export { buildResourceFromType, parseResourcesJson, parseResourcesShorthand }; +export { + buildResourceFromType, + parseResourcesJson, + parseResourcesShorthand, + printNextSteps, +}; diff --git a/packages/shared/src/cli/commands/plugin/create/scaffold.test.ts b/packages/shared/src/cli/commands/plugin/create/scaffold.test.ts index 074ba8a07..ab0d30d49 100644 --- a/packages/shared/src/cli/commands/plugin/create/scaffold.test.ts +++ b/packages/shared/src/cli/commands/plugin/create/scaffold.test.ts @@ -256,6 +256,88 @@ describe("scaffold", () => { }); }); + describe("README.md package manager commands", () => { + it("generates README with pnpm commands when pm is pnpm", () => { + const tmp = makeTempDir(); + tempDirs.push(tmp); + const targetDir = path.join(tmp, "test"); + + scaffoldPlugin(targetDir, BASE_ANSWERS, { isolated: true, pm: "pnpm" }); + + const readme = fs.readFileSync( + path.join(targetDir, "README.md"), + "utf-8", + ); + expect(readme).toContain( + "pnpm add appkit-plugin-my-plugin @databricks/appkit", + ); + }); + + it("generates README with npm commands when pm is npm", () => { + const tmp = makeTempDir(); + tempDirs.push(tmp); + const targetDir = path.join(tmp, "test"); + + scaffoldPlugin(targetDir, BASE_ANSWERS, { isolated: true, pm: "npm" }); + + const readme = fs.readFileSync( + path.join(targetDir, "README.md"), + "utf-8", + ); + expect(readme).toContain( + "npm install appkit-plugin-my-plugin @databricks/appkit", + ); + }); + + it("generates README with yarn commands when pm is yarn", () => { + const tmp = makeTempDir(); + tempDirs.push(tmp); + const targetDir = path.join(tmp, "test"); + + scaffoldPlugin(targetDir, BASE_ANSWERS, { isolated: true, pm: "yarn" }); + + const readme = fs.readFileSync( + path.join(targetDir, "README.md"), + "utf-8", + ); + expect(readme).toContain( + "yarn add appkit-plugin-my-plugin @databricks/appkit", + ); + }); + + it("generates README with bun commands when pm is bun", () => { + const tmp = makeTempDir(); + tempDirs.push(tmp); + const targetDir = path.join(tmp, "test"); + + scaffoldPlugin(targetDir, BASE_ANSWERS, { isolated: true, pm: "bun" }); + + const readme = fs.readFileSync( + path.join(targetDir, "README.md"), + "utf-8", + ); + expect(readme).toContain( + "bun add appkit-plugin-my-plugin @databricks/appkit", + ); + }); + + it("defaults to pnpm when pm is not provided", () => { + const tmp = makeTempDir(); + tempDirs.push(tmp); + const targetDir = path.join(tmp, "test"); + + scaffoldPlugin(targetDir, BASE_ANSWERS, { isolated: true }); + + const readme = fs.readFileSync( + path.join(targetDir, "README.md"), + "utf-8", + ); + expect(readme).toContain( + "pnpm add appkit-plugin-my-plugin @databricks/appkit", + ); + }); + }); + describe("rollback on failure", () => { it("cleans up written files when a write fails partway through", () => { const tmp = makeTempDir(); diff --git a/packages/shared/src/cli/commands/plugin/create/scaffold.ts b/packages/shared/src/cli/commands/plugin/create/scaffold.ts index da7bd04c2..7d9467e2b 100644 --- a/packages/shared/src/cli/commands/plugin/create/scaffold.ts +++ b/packages/shared/src/cli/commands/plugin/create/scaffold.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; +import { PM_COMMANDS, type PackageManager } from "../../../package-manager"; import { humanizeResourceType, MANIFEST_SCHEMA_ID } from "./resource-defaults"; import type { CreateAnswers } from "./types"; @@ -100,7 +101,7 @@ function rollback(written: string[], targetDir: string): void { export function scaffoldPlugin( targetDir: string, answers: CreateAnswers, - options: { isolated: boolean }, + options: { isolated: boolean; pm?: PackageManager }, ): void { fs.mkdirSync(targetDir, { recursive: true }); @@ -203,6 +204,9 @@ export const ${exportName} = toPlugin(${className}); written, ); + const pm = options.pm ?? "pnpm"; + const addCmd = PM_COMMANDS[pm].add(`${packageName} @databricks/appkit`); + const readme = `# ${answers.displayName} ${answers.description} @@ -210,7 +214,7 @@ ${answers.description} ## Installation \`\`\`bash -pnpm add ${packageName} @databricks/appkit +${addCmd} \`\`\` ## Usage diff --git a/packages/shared/src/cli/commands/registry/add.ts b/packages/shared/src/cli/commands/registry/add.ts index d8592e7f5..dbdcb73d7 100644 --- a/packages/shared/src/cli/commands/registry/add.ts +++ b/packages/shared/src/cli/commands/registry/add.ts @@ -6,6 +6,7 @@ import process from "node:process"; import { Command } from "commander"; import pc from "picocolors"; +import { detectPackageManager } from "../../package-manager"; import { fetchRegistryItem, fetchVerifiedNames, @@ -156,13 +157,6 @@ export function pluginExportName(item: RegistryItem): string | null { return chosen && JS_IDENTIFIER.test(chosen) ? chosen : null; } -function detectPackageManager(cwd: string): "pnpm" | "yarn" | "bun" | "npm" { - if (fs.existsSync(path.join(cwd, "pnpm-lock.yaml"))) return "pnpm"; - if (fs.existsSync(path.join(cwd, "yarn.lock"))) return "yarn"; - if (fs.existsSync(path.join(cwd, "bun.lockb"))) return "bun"; - return "npm"; -} - /** * A safe npm dependency spec: `[@scope/]name` with an optional `@version` * range. Registry `dependencies` are untrusted remote data passed to the diff --git a/packages/shared/src/cli/package-manager.test.ts b/packages/shared/src/cli/package-manager.test.ts new file mode 100644 index 000000000..62328a2da --- /dev/null +++ b/packages/shared/src/cli/package-manager.test.ts @@ -0,0 +1,210 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { detectPackageManager, PM_COMMANDS } from "./package-manager"; + +describe("package-manager", () => { + const originalEnv = process.env.npm_config_user_agent; + + beforeEach(() => { + delete process.env.npm_config_user_agent; + }); + + afterEach(() => { + if (originalEnv) { + process.env.npm_config_user_agent = originalEnv; + } else { + delete process.env.npm_config_user_agent; + } + }); + + describe("detectPackageManager", () => { + it("detects pnpm from npm_config_user_agent", () => { + process.env.npm_config_user_agent = "pnpm/8.6.0 npm/? node/18.0.0"; + const cwd = path.join(os.tmpdir(), "test-pnpm"); + expect(detectPackageManager(cwd)).toBe("pnpm"); + }); + + it("detects npm from npm_config_user_agent", () => { + process.env.npm_config_user_agent = "npm/9.8.1 node/18.0.0"; + const cwd = path.join(os.tmpdir(), "test-npm"); + expect(detectPackageManager(cwd)).toBe("npm"); + }); + + it("detects yarn from npm_config_user_agent", () => { + process.env.npm_config_user_agent = "yarn/3.6.0 npm/? node/18.0.0"; + const cwd = path.join(os.tmpdir(), "test-yarn"); + expect(detectPackageManager(cwd)).toBe("yarn"); + }); + + it("detects bun from npm_config_user_agent", () => { + process.env.npm_config_user_agent = "bun/1.0.0 npm/? node/18.0.0"; + const cwd = path.join(os.tmpdir(), "test-bun"); + expect(detectPackageManager(cwd)).toBe("bun"); + }); + + it("falls back to lockfile detection when npm_config_user_agent is absent", () => { + delete process.env.npm_config_user_agent; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "pnpm-lock.yaml"), ""); + expect(detectPackageManager(tmpDir)).toBe("pnpm"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("detects yarn from yarn.lock when env var is absent", () => { + delete process.env.npm_config_user_agent; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "yarn.lock"), ""); + expect(detectPackageManager(tmpDir)).toBe("yarn"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("detects bun from bun.lockb when env var is absent", () => { + delete process.env.npm_config_user_agent; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "bun.lockb"), ""); + expect(detectPackageManager(tmpDir)).toBe("bun"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("detects npm from package-lock.json when env var is absent", () => { + delete process.env.npm_config_user_agent; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "package-lock.json"), ""); + expect(detectPackageManager(tmpDir)).toBe("npm"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("defaults to pnpm when neither env var nor lockfile is present", () => { + delete process.env.npm_config_user_agent; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + expect(detectPackageManager(tmpDir)).toBe("pnpm"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("prefers npm_config_user_agent over lockfile", () => { + process.env.npm_config_user_agent = "npm/9.8.1 node/18.0.0"; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "pnpm-lock.yaml"), ""); + expect(detectPackageManager(tmpDir)).toBe("npm"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("prefers npm_config_user_agent over package-lock.json", () => { + process.env.npm_config_user_agent = "pnpm/8.6.0 npm/? node/18.0.0"; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "package-lock.json"), ""); + expect(detectPackageManager(tmpDir)).toBe("pnpm"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("ignores invalid npm_config_user_agent and falls back to lockfile", () => { + process.env.npm_config_user_agent = "unknown/1.0.0 node/18.0.0"; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + try { + fs.writeFileSync(path.join(tmpDir, "yarn.lock"), ""); + expect(detectPackageManager(tmpDir)).toBe("yarn"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + }); + + describe("PM_COMMANDS", () => { + describe("pnpm", () => { + it("provides correct install command", () => { + expect(PM_COMMANDS.pnpm.install).toBe("pnpm install"); + }); + + it("provides correct build command with run", () => { + expect(PM_COMMANDS.pnpm.build).toBe("pnpm run build"); + }); + + it("provides correct add command", () => { + expect(PM_COMMANDS.pnpm.add("pkg1 pkg2")).toBe("pnpm add pkg1 pkg2"); + }); + + it("provides correct exec command", () => { + expect(PM_COMMANDS.pnpm.exec).toBe("pnpm exec"); + }); + }); + + describe("npm", () => { + it("provides correct install command", () => { + expect(PM_COMMANDS.npm.install).toBe("npm install"); + }); + + it("provides correct build command with run", () => { + expect(PM_COMMANDS.npm.build).toBe("npm run build"); + }); + + it("provides correct add command (uses install)", () => { + expect(PM_COMMANDS.npm.add("pkg1 pkg2")).toBe("npm install pkg1 pkg2"); + }); + + it("provides correct exec command (npx)", () => { + expect(PM_COMMANDS.npm.exec).toBe("npx"); + }); + }); + + describe("yarn", () => { + it("provides correct install command", () => { + expect(PM_COMMANDS.yarn.install).toBe("yarn install"); + }); + + it("provides correct build command with run", () => { + expect(PM_COMMANDS.yarn.build).toBe("yarn run build"); + }); + + it("provides correct add command", () => { + expect(PM_COMMANDS.yarn.add("pkg1 pkg2")).toBe("yarn add pkg1 pkg2"); + }); + + it("provides correct exec command (dlx)", () => { + expect(PM_COMMANDS.yarn.exec).toBe("yarn dlx"); + }); + }); + + describe("bun", () => { + it("provides correct install command", () => { + expect(PM_COMMANDS.bun.install).toBe("bun install"); + }); + + it("provides correct build command with run", () => { + expect(PM_COMMANDS.bun.build).toBe("bun run build"); + }); + + it("provides correct add command", () => { + expect(PM_COMMANDS.bun.add("pkg1 pkg2")).toBe("bun add pkg1 pkg2"); + }); + + it("provides correct exec command (x)", () => { + expect(PM_COMMANDS.bun.exec).toBe("bun x"); + }); + }); + }); +}); diff --git a/packages/shared/src/cli/package-manager.ts b/packages/shared/src/cli/package-manager.ts new file mode 100644 index 000000000..1727ab698 --- /dev/null +++ b/packages/shared/src/cli/package-manager.ts @@ -0,0 +1,78 @@ +import fs from "node:fs"; +import path from "node:path"; + +type PackageManager = "pnpm" | "npm" | "yarn" | "bun"; + +/** + * Detects the package manager for a given working directory. + * Detection order: + * 1. process.env.npm_config_user_agent (e.g. "pnpm/8.6.0 ...") + * 2. Lockfile presence in cwd (pnpm-lock.yaml, yarn.lock, bun.lockb, package-lock.json) + * 3. Default to pnpm + */ +export function detectPackageManager(cwd: string): PackageManager { + // Check environment variable first (npm_config_user_agent set by the package manager) + const userAgent = process.env.npm_config_user_agent; + if (userAgent) { + const firstToken = userAgent.split("/")[0]; + if ( + firstToken === "pnpm" || + firstToken === "npm" || + firstToken === "yarn" || + firstToken === "bun" + ) { + return firstToken; + } + } + + // Check for lockfile presence + if (fs.existsSync(path.join(cwd, "pnpm-lock.yaml"))) return "pnpm"; + if (fs.existsSync(path.join(cwd, "yarn.lock"))) return "yarn"; + if (fs.existsSync(path.join(cwd, "bun.lockb"))) return "bun"; + if (fs.existsSync(path.join(cwd, "package-lock.json"))) return "npm"; + + // Default fallback + return "pnpm"; +} + +/** + * Command strings for each package manager. + * Per-PM forms for: install, build (script), add, exec. + * Scripts always use explicit `run` (e.g., `pnpm run build`). + */ +export const PM_COMMANDS: Record< + PackageManager, + { + install: string; + build: string; + add: (pkgs: string) => string; + exec: string; + } +> = { + pnpm: { + install: "pnpm install", + build: "pnpm run build", + add: (pkgs) => `pnpm add ${pkgs}`, + exec: "pnpm exec", + }, + npm: { + install: "npm install", + build: "npm run build", + add: (pkgs) => `npm install ${pkgs}`, + exec: "npx", + }, + yarn: { + install: "yarn install", + build: "yarn run build", + add: (pkgs) => `yarn add ${pkgs}`, + exec: "yarn dlx", + }, + bun: { + install: "bun install", + build: "bun run build", + add: (pkgs) => `bun add ${pkgs}`, + exec: "bun x", + }, +}; + +export type { PackageManager }; From 4723a378832ed57cc3cc34ffca54184c6c393f19 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Thu, 24 Sep 2026 11:56:08 +0200 Subject: [PATCH 2/2] fix(shared): preserve project package manager during detection Signed-off-by: Atila Fassina --- .../registry/add-package-manager.test.ts | 86 ++++++++ .../shared/src/cli/commands/registry/add.ts | 2 +- .../shared/src/cli/package-manager.test.ts | 195 +++++++----------- packages/shared/src/cli/package-manager.ts | 62 ++++-- 4 files changed, 205 insertions(+), 140 deletions(-) create mode 100644 packages/shared/src/cli/commands/registry/add-package-manager.test.ts diff --git a/packages/shared/src/cli/commands/registry/add-package-manager.test.ts b/packages/shared/src/cli/commands/registry/add-package-manager.test.ts new file mode 100644 index 000000000..065e73b3c --- /dev/null +++ b/packages/shared/src/cli/commands/registry/add-package-manager.test.ts @@ -0,0 +1,86 @@ +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { addCommand } from "./add"; +import { fetchRegistryItem, fetchVerifiedNames } from "./client"; + +vi.mock("node:child_process", async (importOriginal) => ({ + ...(await importOriginal()), + spawnSync: vi.fn(), +})); + +vi.mock("./client", async (importOriginal) => ({ + ...(await importOriginal()), + fetchRegistryItem: vi.fn(), + fetchVerifiedNames: vi.fn(), +})); + +vi.mock("./constants", async (importOriginal) => ({ + ...(await importOriginal()), + resolveToken: () => null, +})); + +describe("registry add package manager", () => { + let cwd: string; + + beforeEach(() => { + cwd = fs.mkdtempSync(path.join(os.tmpdir(), "registry-pm-")); + fs.writeFileSync( + path.join(cwd, "package.json"), + JSON.stringify({ name: "test-app" }), + ); + vi.stubEnv("npm_config_user_agent", undefined); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.mocked(fetchRegistryItem).mockResolvedValue({ + name: "pm-fixture", + dependencies: ["kleur@4.1.5"], + files: [], + }); + vi.mocked(fetchVerifiedNames).mockResolvedValue(new Set(["pm-fixture"])); + vi.mocked(spawnSync).mockReturnValue({ + pid: 0, + output: [], + stdout: "", + stderr: "", + status: 0, + signal: null, + }); + }); + + afterEach(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + vi.restoreAllMocks(); + vi.clearAllMocks(); + vi.unstubAllEnvs(); + }); + + it.each([ + ["pnpm-lock.yaml", "npm/11.8.0", "pnpm"], + ["yarn.lock", "npm/11.8.0", "yarn"], + ["package-lock.json", "pnpm/11.0.8", "npm"], + ["npm-shrinkwrap.json", undefined, "npm"], + ["bun.lock", undefined, "bun"], + ["bun.lockb", undefined, "bun"], + [null, undefined, "npm"], + ] as const)( + "installs with %s and launcher %s using %s", + async (lockfile, userAgent, pm) => { + if (lockfile) fs.writeFileSync(path.join(cwd, lockfile), ""); + vi.stubEnv("npm_config_user_agent", userAgent); + + await addCommand.parseAsync(["pm-fixture", "--cwd", cwd, "--yes"], { + from: "user", + }); + + expect(spawnSync).toHaveBeenCalledExactlyOnceWith( + pm, + [pm === "npm" ? "install" : "add", "--", "kleur@4.1.5"], + { cwd, stdio: "inherit" }, + ); + }, + ); +}); diff --git a/packages/shared/src/cli/commands/registry/add.ts b/packages/shared/src/cli/commands/registry/add.ts index dbdcb73d7..e6fc52946 100644 --- a/packages/shared/src/cli/commands/registry/add.ts +++ b/packages/shared/src/cli/commands/registry/add.ts @@ -295,7 +295,7 @@ async function installDependencies(deps: string[], cwd: string): Promise { } if (install.length === 0) return; - const pm = detectPackageManager(cwd); + const pm = detectPackageManager(cwd, "npm"); const subcommand = pm === "npm" ? "install" : "add"; console.log(`\nInstalling dependencies with ${pm}: ${install.join(" ")}`); // `--` stops the PM from parsing any dep as a flag (defense in depth). diff --git a/packages/shared/src/cli/package-manager.test.ts b/packages/shared/src/cli/package-manager.test.ts index 62328a2da..9741c792b 100644 --- a/packages/shared/src/cli/package-manager.test.ts +++ b/packages/shared/src/cli/package-manager.test.ts @@ -2,135 +2,96 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { detectPackageManager, PM_COMMANDS } from "./package-manager"; describe("package-manager", () => { - const originalEnv = process.env.npm_config_user_agent; - - beforeEach(() => { - delete process.env.npm_config_user_agent; - }); - - afterEach(() => { - if (originalEnv) { - process.env.npm_config_user_agent = originalEnv; - } else { - delete process.env.npm_config_user_agent; - } - }); - describe("detectPackageManager", () => { - it("detects pnpm from npm_config_user_agent", () => { - process.env.npm_config_user_agent = "pnpm/8.6.0 npm/? node/18.0.0"; - const cwd = path.join(os.tmpdir(), "test-pnpm"); - expect(detectPackageManager(cwd)).toBe("pnpm"); - }); - - it("detects npm from npm_config_user_agent", () => { - process.env.npm_config_user_agent = "npm/9.8.1 node/18.0.0"; - const cwd = path.join(os.tmpdir(), "test-npm"); + let cwd: string; + + beforeEach(() => { + cwd = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); + vi.stubEnv("npm_config_user_agent", undefined); + }); + + afterEach(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + vi.unstubAllEnvs(); + }); + + it.each(["pnpm", "npm", "yarn", "bun"] as const)( + "uses %s from the launcher when there is no project metadata", + (pm) => { + vi.stubEnv("npm_config_user_agent", `${pm}/1.0.0 node/24.0.0`); + expect(detectPackageManager(cwd)).toBe(pm); + }, + ); + + it.each(["pnpm", "npm", "yarn", "bun"] as const)( + "prefers declared %s over conflicting lockfiles and launcher", + (pm) => { + fs.writeFileSync( + path.join(cwd, "package.json"), + JSON.stringify({ packageManager: `${pm}@1.0.0` }), + ); + fs.writeFileSync(path.join(cwd, "pnpm-lock.yaml"), ""); + fs.writeFileSync(path.join(cwd, "package-lock.json"), "{}"); + vi.stubEnv( + "npm_config_user_agent", + pm === "npm" ? "pnpm/11.0.8" : "npm/11.8.0", + ); + expect(detectPackageManager(cwd)).toBe(pm); + }, + ); + + it.each([ + ["pnpm-lock.yaml", "pnpm"], + ["yarn.lock", "yarn"], + ["bun.lock", "bun"], + ["bun.lockb", "bun"], + ["package-lock.json", "npm"], + ["npm-shrinkwrap.json", "npm"], + ] as const)( + "detects %s with an absent or conflicting launcher", + (lockfile, pm) => { + fs.writeFileSync(path.join(cwd, lockfile), ""); + expect(detectPackageManager(cwd)).toBe(pm); + + vi.stubEnv( + "npm_config_user_agent", + pm === "npm" ? "pnpm/11.0.8" : "npm/11.8.0", + ); + expect(detectPackageManager(cwd)).toBe(pm); + }, + ); + + it.each([ + "invalid json", + "null", + "{}", + '{"packageManager":null}', + '{"packageManager":42}', + '{"packageManager":"unknown@1.0.0"}', + ])("falls back to lockfiles for unusable package.json: %s", (content) => { + fs.writeFileSync(path.join(cwd, "package.json"), content); + fs.writeFileSync(path.join(cwd, "package-lock.json"), "{}"); + vi.stubEnv("npm_config_user_agent", "pnpm/11.0.8"); expect(detectPackageManager(cwd)).toBe("npm"); }); - it("detects yarn from npm_config_user_agent", () => { - process.env.npm_config_user_agent = "yarn/3.6.0 npm/? node/18.0.0"; - const cwd = path.join(os.tmpdir(), "test-yarn"); - expect(detectPackageManager(cwd)).toBe("yarn"); - }); - - it("detects bun from npm_config_user_agent", () => { - process.env.npm_config_user_agent = "bun/1.0.0 npm/? node/18.0.0"; - const cwd = path.join(os.tmpdir(), "test-bun"); - expect(detectPackageManager(cwd)).toBe("bun"); - }); - - it("falls back to lockfile detection when npm_config_user_agent is absent", () => { - delete process.env.npm_config_user_agent; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "pnpm-lock.yaml"), ""); - expect(detectPackageManager(tmpDir)).toBe("pnpm"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); - - it("detects yarn from yarn.lock when env var is absent", () => { - delete process.env.npm_config_user_agent; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "yarn.lock"), ""); - expect(detectPackageManager(tmpDir)).toBe("yarn"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); - - it("detects bun from bun.lockb when env var is absent", () => { - delete process.env.npm_config_user_agent; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "bun.lockb"), ""); - expect(detectPackageManager(tmpDir)).toBe("bun"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); - - it("detects npm from package-lock.json when env var is absent", () => { - delete process.env.npm_config_user_agent; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "package-lock.json"), ""); - expect(detectPackageManager(tmpDir)).toBe("npm"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); - - it("defaults to pnpm when neither env var nor lockfile is present", () => { - delete process.env.npm_config_user_agent; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - expect(detectPackageManager(tmpDir)).toBe("pnpm"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); - - it("prefers npm_config_user_agent over lockfile", () => { - process.env.npm_config_user_agent = "npm/9.8.1 node/18.0.0"; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "pnpm-lock.yaml"), ""); - expect(detectPackageManager(tmpDir)).toBe("npm"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + it("defaults to pnpm without project or launcher metadata", () => { + expect(detectPackageManager(cwd)).toBe("pnpm"); }); - it("prefers npm_config_user_agent over package-lock.json", () => { - process.env.npm_config_user_agent = "pnpm/8.6.0 npm/? node/18.0.0"; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "package-lock.json"), ""); - expect(detectPackageManager(tmpDir)).toBe("pnpm"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + it("allows callers to retain an npm fallback", () => { + expect(detectPackageManager(cwd, "npm")).toBe("npm"); }); - it("ignores invalid npm_config_user_agent and falls back to lockfile", () => { - process.env.npm_config_user_agent = "unknown/1.0.0 node/18.0.0"; - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "pm-detect-")); - try { - fs.writeFileSync(path.join(tmpDir, "yarn.lock"), ""); - expect(detectPackageManager(tmpDir)).toBe("yarn"); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } + it("ignores unknown launchers", () => { + vi.stubEnv("npm_config_user_agent", "unknown/1.0.0 node/24.0.0"); + expect(detectPackageManager(cwd)).toBe("pnpm"); + expect(detectPackageManager(cwd, "npm")).toBe("npm"); }); }); diff --git a/packages/shared/src/cli/package-manager.ts b/packages/shared/src/cli/package-manager.ts index 1727ab698..49b34335d 100644 --- a/packages/shared/src/cli/package-manager.ts +++ b/packages/shared/src/cli/package-manager.ts @@ -3,36 +3,54 @@ import path from "node:path"; type PackageManager = "pnpm" | "npm" | "yarn" | "bun"; +function isPackageManager(value: string): value is PackageManager { + return ( + value === "pnpm" || value === "npm" || value === "yarn" || value === "bun" + ); +} + /** - * Detects the package manager for a given working directory. - * Detection order: - * 1. process.env.npm_config_user_agent (e.g. "pnpm/8.6.0 ...") - * 2. Lockfile presence in cwd (pnpm-lock.yaml, yarn.lock, bun.lockb, package-lock.json) - * 3. Default to pnpm + * Prefers the project's packageManager field, then its lockfiles, then + * npm_config_user_agent. The launcher (e.g. npx) may use a different manager. */ -export function detectPackageManager(cwd: string): PackageManager { - // Check environment variable first (npm_config_user_agent set by the package manager) - const userAgent = process.env.npm_config_user_agent; - if (userAgent) { - const firstToken = userAgent.split("/")[0]; - if ( - firstToken === "pnpm" || - firstToken === "npm" || - firstToken === "yarn" || - firstToken === "bun" - ) { - return firstToken; +export function detectPackageManager( + cwd: string, + fallback: PackageManager = "pnpm", +): PackageManager { + try { + const pkg = JSON.parse( + fs.readFileSync(path.join(cwd, "package.json"), "utf-8"), + ) as { packageManager?: unknown } | null; + if (typeof pkg?.packageManager === "string") { + const name = pkg.packageManager.split("@")[0]; + if (isPackageManager(name)) return name; } + } catch { + // A missing or unreadable manifest still permits lockfile detection. } - // Check for lockfile presence if (fs.existsSync(path.join(cwd, "pnpm-lock.yaml"))) return "pnpm"; if (fs.existsSync(path.join(cwd, "yarn.lock"))) return "yarn"; - if (fs.existsSync(path.join(cwd, "bun.lockb"))) return "bun"; - if (fs.existsSync(path.join(cwd, "package-lock.json"))) return "npm"; + if ( + fs.existsSync(path.join(cwd, "bun.lock")) || + fs.existsSync(path.join(cwd, "bun.lockb")) + ) { + return "bun"; + } + if ( + fs.existsSync(path.join(cwd, "package-lock.json")) || + fs.existsSync(path.join(cwd, "npm-shrinkwrap.json")) + ) { + return "npm"; + } + + const userAgent = process.env.npm_config_user_agent; + if (userAgent) { + const firstToken = userAgent.split("/")[0]; + if (isPackageManager(firstToken)) return firstToken; + } - // Default fallback - return "pnpm"; + return fallback; } /**