Merge pull request #190 from cloudflare/cina/180-fix-undefined-package-manager

Use npm as fallback if no `packageManager` specified

.changeset/slimy-cameras-play.md

@@ -0,0 +1,5 @@
+---
+"wrangler-action": patch
+---
+
+Fixed action failure when no `packageManager` specified and no lockfile is found. The action now falls back to using npm.

.github/workflows/deploy.yml

@@ -122,6 +122,14 @@ jobs:
           accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
           command: deploy --dry-run
 
+      - name: Support unspecified packageManager with no lockfile
+        uses: ./
+        with:
+          workingDirectory: "./test/empty"
+          apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }}
+          accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
+          command: deploy --dry-run
+
       - name: Support npm package manager
         uses: ./
         with:

action.yml

@@ -42,5 +42,6 @@ inputs:
     description: "A string of environment variable names, separated by newlines. These will be bound to your Worker using the values of matching environment variables declared in `env` of this workflow."
     required: false
   packageManager:
-    description: "The name of the package manager to install and run wrangler. If not provided, it will be detected via the lock file. Valid values: [npm, pnpm, yarn]"
+    description: "The package manager you'd like to use to install and run wrangler. If not specified, a value will be inferred based on the presence of a lockfile. Valid values: [npm, pnpm, yarn]"
     required: false
+    default: npm

src/index.ts

@@ -10,37 +10,12 @@ import {
 } from "@actions/core";
 import { exec, execSync } from "node:child_process";
 import * as util from "node:util";
-import {
-	PackageManager,
-	checkWorkingDirectory,
-	detectPackageManager,
-	isValidPackageManager,
-	semverCompare,
-} from "./utils";
+import { checkWorkingDirectory, semverCompare } from "./utils";
+import { getPackageManager } from "./packageManagers";
 const execAsync = util.promisify(exec);
 
 const DEFAULT_WRANGLER_VERSION = "3.5.1";
 
-interface PackageManagerCommands {
-	install: string;
-	exec: string;
-}
-
-const PACKAGE_MANAGER_COMMANDS = {
-	npm: {
-		install: "npm i",
-		exec: "npx",
-	},
-	yarn: {
-		install: "yarn add",
-		exec: "yarn",
-	},
-	pnpm: {
-		install: "pnpm add",
-		exec: "pnpm exec",
-	},
-} as const satisfies Readonly<Record<PackageManager, PackageManagerCommands>>;
-
 /**
  * A configuration object that contains all the inputs & immutable state for the action.
  */
@@ -57,20 +32,9 @@ const config = {
 	PACKAGE_MANAGER: getInput("packageManager"),
 } as const;
 
-function realPackageManager(): PackageManager {
-	if (isValidPackageManager(config.PACKAGE_MANAGER)) {
-		return config.PACKAGE_MANAGER;
-	}
-
-	const packageManager = detectPackageManager(config.workingDirectory);
-	if (packageManager !== null) {
-		return packageManager;
-	}
-
-	throw new Error("Package manager is not detected");
-}
-
-const pkgManagerCmd = PACKAGE_MANAGER_COMMANDS[realPackageManager()];
+const packageManager = getPackageManager(config.PACKAGE_MANAGER, {
+	workingDirectory: config.workingDirectory,
+});
 
 function info(message: string, bypass?: boolean): void {
 	if (!config.QUIET_MODE || bypass) {
@@ -136,7 +100,7 @@ function installWrangler() {
 		);
 	}
 	startGroup("📥 Installing Wrangler");
-	const command = `${pkgManagerCmd.install} wrangler@${config["WRANGLER_VERSION"]}`;
+	const command = `${packageManager.install} wrangler@${config["WRANGLER_VERSION"]}`;
 	info(`Running command: ${command}`);
 	execSync(command, { cwd: config["workingDirectory"], env: process.env });
 	info(`✅ Wrangler installed`, true);
@@ -157,7 +121,7 @@ async function execCommands(commands: string[], cmdType: string) {
 	try {
 		const arrPromises = commands.map(async (command) => {
 			const cmd = command.startsWith("wrangler")
-				? `${pkgManagerCmd.exec} ${command}`
+				? `${packageManager.exec} ${command}`
 				: command;
 
 			info(`🚀 Executing command: ${cmd}`);
@@ -198,7 +162,7 @@ async function legacyUploadSecrets(
 	const arrPromises = secrets
 		.map((secret) => {
 			const command = `echo ${getSecret(secret)} | ${
-				pkgManagerCmd.exec
+				packageManager.exec
 			} wrangler secret put ${secret}`;
 			return environment ? command.concat(` --env ${environment}`) : command;
 		})
@@ -240,7 +204,7 @@ async function uploadSecrets() {
 		const secretCmd = `echo "${JSON.stringify(secretObj).replaceAll(
 			'"',
 			'\\"',
-		)}" | ${pkgManagerCmd.exec} wrangler secret:bulk ${environmentSuffix}`;
+		)}" | ${packageManager.exec} wrangler secret:bulk ${environmentSuffix}`;
 
 		execSync(secretCmd, {
 			cwd: workingDirectory,
@@ -289,7 +253,7 @@ async function wranglerCommands() {
 				command = command.concat(` --env ${environment}`);
 			}
 
-			const cmd = `${pkgManagerCmd.exec} wrangler ${command} ${
+			const cmd = `${packageManager.exec} wrangler ${command} ${
 				(command.startsWith("deploy") || command.startsWith("publish")) &&
 				!command.includes(`--var`)
 					? getVarArgs()

src/packageManagers.test.ts

@@ -0,0 +1,76 @@
+import { describe, expect, test } from "vitest";
+import { getPackageManager } from "./packageManagers";
+
+describe("getPackageManager", () => {
+	test("should use provided value instead of inferring from lockfile", () => {
+		expect(getPackageManager("npm", { workingDirectory: "test/npm" }))
+			.toMatchInlineSnapshot(`
+				{
+				  "exec": "npx",
+				  "install": "npm i",
+				}
+			`);
+
+		expect(getPackageManager("yarn", { workingDirectory: "test/npm" }))
+			.toMatchInlineSnapshot(`
+			{
+			  "exec": "yarn",
+			  "install": "yarn add",
+			}
+		`);
+
+		expect(getPackageManager("pnpm", { workingDirectory: "test/npm" }))
+			.toMatchInlineSnapshot(`
+				{
+				  "exec": "pnpm exec",
+				  "install": "pnpm add",
+				}
+			`);
+	});
+
+	test("should use npm if no value provided and package-lock.json exists", () => {
+		expect(getPackageManager("", { workingDirectory: "test/npm" }))
+			.toMatchInlineSnapshot(`
+			{
+			  "exec": "npx",
+			  "install": "npm i",
+			}
+		`);
+	});
+
+	test("should use yarn if no value provided and yarn.lock exists", () => {
+		expect(getPackageManager("", { workingDirectory: "test/yarn" }))
+			.toMatchInlineSnapshot(`
+			{
+			  "exec": "yarn",
+			  "install": "yarn add",
+			}
+		`);
+	});
+
+	test("should use pnpm if no value provided and pnpm-lock.yaml exists", () => {
+		expect(getPackageManager("", { workingDirectory: "test/pnpm" }))
+			.toMatchInlineSnapshot(`
+			{
+			  "exec": "pnpm exec",
+			  "install": "pnpm add",
+			}
+		`);
+	});
+
+	test("should use npm if no value provided and no lockfile is present", () => {
+		expect(getPackageManager("", { workingDirectory: "test/empty" }))
+			.toMatchInlineSnapshot(`
+			{
+			  "exec": "npx",
+			  "install": "npm i",
+			}
+		`);
+	});
+
+	test("should throw if an invalid value is provided", () => {
+		expect(() =>
+			getPackageManager("cargo", { workingDirectory: "test/npm" }),
+		).toThrowError();
+	});
+});

src/packageManagers.ts

@@ -0,0 +1,61 @@
+import { existsSync } from "node:fs";
+import * as path from "node:path";
+
+interface PackageManager {
+	install: string;
+	exec: string;
+}
+
+const PACKAGE_MANAGERS = {
+	npm: {
+		install: "npm i",
+		exec: "npx",
+	},
+	yarn: {
+		install: "yarn add",
+		exec: "yarn",
+	},
+	pnpm: {
+		install: "pnpm add",
+		exec: "pnpm exec",
+	},
+} as const satisfies Readonly<Record<string, PackageManager>>;
+
+type PackageManagerValue = keyof typeof PACKAGE_MANAGERS;
+
+function detectPackageManager(
+	workingDirectory = ".",
+): PackageManagerValue | null {
+	if (existsSync(path.join(workingDirectory, "package-lock.json"))) {
+		return "npm";
+	}
+	if (existsSync(path.join(workingDirectory, "yarn.lock"))) {
+		return "yarn";
+	}
+	if (existsSync(path.join(workingDirectory, "pnpm-lock.yaml"))) {
+		return "pnpm";
+	}
+	return null;
+}
+
+function assertValidPackageManagerValue(
+	name: string,
+): asserts name is PackageManagerValue | "" {
+	if (name && !Object.keys(PACKAGE_MANAGERS).includes(name)) {
+		throw new TypeError(
+			`invalid value provided for "packageManager": ${name}
+	Value must be one of: [${Object.keys(PACKAGE_MANAGERS).join(", ")}]`,
+		);
+	}
+}
+
+export function getPackageManager(
+	name: string,
+	{ workingDirectory = "." }: { workingDirectory?: string } = {},
+) {
+	assertValidPackageManagerValue(name);
+
+	return PACKAGE_MANAGERS[
+		name || detectPackageManager(workingDirectory) || "npm"
+	];
+}

src/utils.test.ts

@@ -1,11 +1,6 @@
 import path from "node:path";
 import { describe, expect, test } from "vitest";
-import {
-	checkWorkingDirectory,
-	detectPackageManager,
-	isValidPackageManager,
-	semverCompare,
-} from "./utils";
+import { checkWorkingDirectory, semverCompare } from "./utils";
 
 describe("semverCompare", () => {
 	test("should return false if the second argument is equal to the first argument", () => {
@@ -35,34 +30,3 @@ describe("checkWorkingDirectory", () => {
 		);
 	});
 });
-
-describe("detectPackageManager", () => {
-	test("should return name of package manager for current workspace", () => {
-		expect(detectPackageManager()).toBe("npm");
-	});
-
-	test("should return npm if package-lock.json exists", () => {
-		expect(detectPackageManager("test/npm")).toBe("npm");
-	});
-
-	test("should return yarn if yarn.lock exists", () => {
-		expect(detectPackageManager("test/yarn")).toBe("yarn");
-	});
-
-	test("should return pnpm if pnpm-lock.yaml exists", () => {
-		expect(detectPackageManager("test/pnpm")).toBe("pnpm");
-	});
-
-	test("should return null if no package manager is detected", () => {
-		expect(detectPackageManager("test/empty")).toBe(null);
-	});
-});
-
-test("isValidPackageManager", () => {
-	expect(isValidPackageManager("npm")).toBe(true);
-	expect(isValidPackageManager("pnpm")).toBe(true);
-	expect(isValidPackageManager("yarn")).toBe(true);
-
-	expect(isValidPackageManager("")).toBe(false);
-	expect(isValidPackageManager("ppnpm")).toBe(false);
-});

src/utils.ts

@@ -29,24 +29,3 @@ export function checkWorkingDirectory(workingDirectory = ".") {
 		throw new Error(`Directory ${workingDirectory} does not exist.`);
 	}
 }
-
-export type PackageManager = "npm" | "yarn" | "pnpm";
-
-export function detectPackageManager(
-	workingDirectory = ".",
-): PackageManager | null {
-	if (existsSync(path.join(workingDirectory, "package-lock.json"))) {
-		return "npm";
-	}
-	if (existsSync(path.join(workingDirectory, "yarn.lock"))) {
-		return "yarn";
-	}
-	if (existsSync(path.join(workingDirectory, "pnpm-lock.yaml"))) {
-		return "pnpm";
-	}
-	return null;
-}
-
-export function isValidPackageManager(name: string): name is PackageManager {
-	return name === "npm" || name === "yarn" || name === "pnpm";
-}