Skip to content

Commit bbc02d7

Browse files
fix: detect missing package managers
spawnSync returns a result instead of throwing, so the CLI always chose npm without a lock file, even when npm was missing. Check the error and the exit status, so yarn and pnpm are found too. Claude-Session: https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
1 parent e76d9ab commit bbc02d7

2 files changed

Lines changed: 32 additions & 32 deletions

File tree

‎packages/webpack-cli/src/webpack-cli.ts‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -730,37 +730,18 @@ class WebpackCLI {
730730
// Nothing
731731
}
732732

733-
try {
734-
// the sync function below will fail if npm is not installed,
735-
// an error will be thrown
736-
if (await this.#spawnSync("npm", ["--version"])) {
737-
return "npm";
738-
}
739-
} catch {
740-
// Nothing
741-
}
733+
// A missing package manager fails to start, or exits with an error in the shell on Windows
734+
for (const packageManager of ["npm", "yarn", "pnpm"] as const) {
735+
const { error, status } = await this.#spawnSync(packageManager, ["--version"]);
742736

743-
try {
744-
// the sync function below will fail if yarn is not installed,
745-
// an error will be thrown
746-
if (await this.#spawnSync("yarn", ["--version"])) {
747-
return "yarn";
737+
if (!error && status === 0) {
738+
return packageManager;
748739
}
749-
} catch {
750-
// Nothing
751740
}
752741

753-
try {
754-
// the sync function below will fail if pnpm is not installed,
755-
// an error will be thrown
756-
if (await this.#spawnSync("pnpm", ["--version"])) {
757-
return "pnpm";
758-
}
759-
} catch {
760-
this.logger.error("No package manager found.");
742+
this.logger.error("No package manager found.");
761743

762-
process.exit(2);
763-
}
744+
process.exit(2);
764745
}
765746

766747
async isPackageInstalled(packageName: string): Promise<boolean> {

‎test/api/get-default-package-manager.test.js‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ const fs = require("node:fs");
22
const path = require("node:path");
33
const CLI = require("../../packages/webpack-cli/lib/webpack-cli").default;
44

5-
const syncMock = jest.fn(() => ({
6-
stdout: "1.0.0",
7-
}));
5+
const installed = () => ({ status: 0, stdout: "1.0.0" });
6+
const missing = () => ({ error: Object.assign(new Error("spawnSync ENOENT"), { code: "ENOENT" }) });
7+
const syncMock = jest.fn(installed);
88
jest.unstable_mockModule("node:child_process", () => ({ spawnSync: syncMock }));
99

1010
describe("getPackageManager", () => {
@@ -38,6 +38,7 @@ describe("getPackageManager", () => {
3838
cli = new CLI();
3939

4040
syncMock.mockClear();
41+
syncMock.mockImplementation(installed);
4142
});
4243

4344
it("should find yarn.lock", async () => {
@@ -96,11 +97,26 @@ describe("getPackageManager", () => {
9697
expect(syncMock).toHaveBeenCalledTimes(1);
9798
});
9899

100+
it("should find yarn when npm is missing", async () => {
101+
cwdSpy.mockReturnValue(noLockPath);
102+
syncMock.mockImplementationOnce(missing);
103+
104+
expect(await cli.getDefaultPackageManager()).toBe("yarn");
105+
expect(syncMock).toHaveBeenCalledTimes(2);
106+
});
107+
108+
it("should find pnpm when npm fails and yarn is missing", async () => {
109+
cwdSpy.mockReturnValue(noLockPath);
110+
// On Windows a missing command fails in the shell
111+
syncMock.mockReturnValueOnce({ status: 1 }).mockImplementationOnce(missing);
112+
113+
expect(await cli.getDefaultPackageManager()).toBe("pnpm");
114+
expect(syncMock).toHaveBeenCalledTimes(3);
115+
});
116+
99117
it("should throw error if no package manager is found", async () => {
100118
cwdSpy.mockReturnValue(noLockPath);
101-
syncMock.mockImplementation(() => {
102-
throw new Error("error");
103-
});
119+
syncMock.mockImplementation(missing);
104120
const mockExit = jest.spyOn(process, "exit").mockImplementation(() => {});
105121
// Do not print warning in CI
106122
const consoleMock = jest.spyOn(console, "error").mockImplementation(() => {});
@@ -109,5 +125,8 @@ describe("getPackageManager", () => {
109125
expect(mockExit).toHaveBeenCalledWith(2);
110126
expect(consoleMock).toHaveBeenCalledTimes(1);
111127
expect(syncMock).toHaveBeenCalledTimes(3); // 3 calls for npm, yarn and pnpm
128+
129+
mockExit.mockRestore();
130+
consoleMock.mockRestore();
112131
});
113132
});

0 commit comments

Comments
 (0)