From 528687aaf436f67565918533ffd15c250f39c47b Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Mon, 9 Oct 2023 17:20:09 -0500 Subject: [PATCH] Refactor to use npm as a fallback if no `packageManager` specified... and no lockfile is present. Fixes #180 --- .changeset/slimy-cameras-play.md | 5 +++ .github/workflows/deploy.yml | 8 ++++ action.yml | 3 +- src/index.ts | 56 +++++------------------ src/packageManagers.test.ts | 76 ++++++++++++++++++++++++++++++++ src/packageManagers.ts | 61 +++++++++++++++++++++++++ src/utils.test.ts | 38 +--------------- src/utils.ts | 21 --------- 8 files changed, 163 insertions(+), 105 deletions(-) create mode 100644 .changeset/slimy-cameras-play.md create mode 100644 src/packageManagers.test.ts create mode 100644 src/packageManagers.ts diff --git a/.changeset/slimy-cameras-play.md b/.changeset/slimy-cameras-play.md new file mode 100644 index 0000000..7086cda --- /dev/null +++ b/.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. diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index cdabf99..a23b5e4 100644 --- a/.github/workflows/deploy.yml +++ b/.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: diff --git a/action.yml b/action.yml index e9e9b1d..6974b31 100644 --- a/action.yml +++ b/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 diff --git a/src/index.ts b/src/index.ts index 251a594..5e38275 100755 --- a/src/index.ts +++ b/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>; - /** * 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() diff --git a/src/packageManagers.test.ts b/src/packageManagers.test.ts new file mode 100644 index 0000000..9e97032 --- /dev/null +++ b/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(); + }); +}); diff --git a/src/packageManagers.ts b/src/packageManagers.ts new file mode 100644 index 0000000..b395932 --- /dev/null +++ b/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>; + +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" + ]; +} diff --git a/src/utils.test.ts b/src/utils.test.ts index e02c84d..658a29f 100644 --- a/src/utils.test.ts +++ b/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); -}); diff --git a/src/utils.ts b/src/utils.ts index c644425..ce6392b 100644 --- a/src/utils.ts +++ b/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"; -}