fix(cli-v3): ignore engine checks during deployment install to prevent build server failures #2990
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@trigger.dev/cli-v3": patch | ||
| --- | ||
|
|
||
| Fix: Ignore engine checks during deployment install phase to prevent failure on build server when Node version mismatch exists. (#2913) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
|
|
||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; | ||
| import { updateTriggerPackages } from "./update.js"; | ||
| import * as nypm from "nypm"; | ||
| import * as pkgTypes from "pkg-types"; | ||
| import * as fs from "node:fs/promises"; | ||
| import * as clack from "@clack/prompts"; | ||
| import path from "node:path"; | ||
|
|
||
| // Mock dependencies | ||
| vi.mock("nypm"); | ||
| vi.mock("pkg-types"); | ||
| vi.mock("node:fs/promises"); | ||
| vi.mock("@clack/prompts"); | ||
| vi.mock("std-env", () => ({ | ||
| hasTTY: true, | ||
| isCI: false, | ||
| })); | ||
| vi.mock("../utilities/initialBanner.js", () => ({ | ||
| updateCheck: vi.fn().mockResolvedValue(undefined), | ||
| printStandloneInitialBanner: vi.fn(), | ||
| })); | ||
| vi.mock("../version.js", () => ({ | ||
| VERSION: "3.0.0", | ||
| })); | ||
| vi.mock("../cli/common.js", () => ({ | ||
| CommonCommandOptions: { pick: () => ({}) }, | ||
| })); | ||
| vi.mock("../utilities/cliOutput.js", () => ({ | ||
| chalkError: vi.fn(), | ||
| prettyError: vi.fn(), | ||
| prettyWarning: vi.fn(), | ||
| })); | ||
| vi.mock("../utilities/fileSystem.js", () => ({ | ||
| removeFile: vi.fn(), | ||
| writeJSONFilePreserveOrder: vi.fn(), | ||
| })); | ||
| vi.mock("../utilities/logger.js", () => ({ | ||
| logger: { | ||
| debug: vi.fn(), | ||
| log: vi.fn(), | ||
| table: vi.fn(), | ||
| }, | ||
| })); | ||
| vi.mock("../utilities/windows.js", () => ({ | ||
| spinner: () => ({ | ||
| start: vi.fn(), | ||
| message: vi.fn(), | ||
| stop: vi.fn(), | ||
| }), | ||
| })); | ||
|
|
||
| describe("updateTriggerPackages", () => { | ||
| beforeEach(() => { | ||
| vi.resetAllMocks(); | ||
|
|
||
| // Default mocks | ||
| vi.mocked(fs.writeFile).mockResolvedValue(undefined); | ||
| vi.mocked(fs.rm).mockResolvedValue(undefined); | ||
| vi.mocked(pkgTypes.readPackageJSON).mockResolvedValue({ | ||
| dependencies: { | ||
| "@trigger.dev/sdk": "2.0.0", // Mismatch | ||
| }, | ||
| }); | ||
| vi.mocked(pkgTypes.resolvePackageJSON).mockResolvedValue("/path/to/package.json"); | ||
| vi.mocked(clack.confirm).mockResolvedValue(true); // User confirms update | ||
| vi.mocked(nypm.installDependencies).mockResolvedValue(undefined); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("should pass --no-engine-strict for npm when ignoreEngines is true", async () => { | ||
| vi.mocked(nypm.detectPackageManager).mockResolvedValue({ name: "npm", command: "npm", version: "1.0.0" } as any); | ||
|
|
||
| await updateTriggerPackages(".", { ignoreEngines: true } as any, true, true); | ||
|
|
||
| expect(nypm.installDependencies).toHaveBeenCalledWith(expect.objectContaining({ | ||
| args: ["--no-engine-strict"], | ||
| })); | ||
| }); | ||
|
|
||
| it("should pass --config.engine-strict=false for pnpm when ignoreEngines is true", async () => { | ||
| vi.mocked(nypm.detectPackageManager).mockResolvedValue({ name: "pnpm", command: "pnpm", version: "1.0.0" } as any); | ||
|
|
||
| await updateTriggerPackages(".", { ignoreEngines: true } as any, true, true); | ||
|
|
||
| expect(nypm.installDependencies).toHaveBeenCalledWith(expect.objectContaining({ | ||
| args: ["--config.engine-strict=false"], | ||
| })); | ||
| }); | ||
|
|
||
| it("should pass --ignore-engines for yarn when ignoreEngines is true", async () => { | ||
| vi.mocked(nypm.detectPackageManager).mockResolvedValue({ name: "yarn", command: "yarn", version: "1.0.0" } as any); | ||
|
|
||
| await updateTriggerPackages(".", { ignoreEngines: true } as any, true, true); | ||
|
|
||
| expect(nypm.installDependencies).toHaveBeenCalledWith(expect.objectContaining({ | ||
| args: ["--ignore-engines"], | ||
| })); | ||
| }); | ||
|
|
||
| it("should NOT pass engine flags if ignoreEngines is false (default)", async () => { | ||
| vi.mocked(nypm.detectPackageManager).mockResolvedValue({ name: "npm", command: "npm", version: "1.0.0" } as any); | ||
|
|
||
| await updateTriggerPackages(".", { ignoreEngines: false } as any, true, true); | ||
|
|
||
| expect(nypm.installDependencies).toHaveBeenCalledWith(expect.objectContaining({ | ||
| args: [], | ||
| })); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ import * as semver from "semver"; | |
| export const UpdateCommandOptions = CommonCommandOptions.pick({ | ||
| logLevel: true, | ||
| skipTelemetry: true, | ||
| ignoreEngines: true, | ||
|
There was a problem hiding this comment. 🟡 ignoreEngines property picked from schema that doesn't contain it The Click to expandIssueIn export const UpdateCommandOptions = CommonCommandOptions.pick({
logLevel: true,
skipTelemetry: true,
ignoreEngines: true, // This doesn't exist in CommonCommandOptions!
});But looking at export const CommonCommandOptions = z.object({
apiUrl: z.string().optional(),
logLevel: z.enum(["debug", "info", "log", "warn", "error", "none"]).default("log"),
skipTelemetry: z.boolean().default(false),
profile: z.string().default(readAuthConfigCurrentProfileName()),
});Zod's ImpactWhen RecommendationAdd Recommendation: Either add 
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| }); | ||
|
|
||
| export type UpdateCommandOptions = z.infer<typeof UpdateCommandOptions>; | ||
|
|
@@ -257,11 +258,26 @@ export async function updateTriggerPackages( | |
| `Installing new package versions${packageManager ? ` with ${packageManager.name}` : ""}` | ||
| ); | ||
|
|
||
| const installArgs: string[] = []; | ||
|
|
||
| if (options.ignoreEngines && packageManager) { | ||
| switch (packageManager.name) { | ||
| case "npm": | ||
| installArgs.push("--no-engine-strict"); | ||
| break; | ||
| case "pnpm": | ||
| installArgs.push("--config.engine-strict=false"); | ||
| break; | ||
| case "yarn": | ||
| installArgs.push("--ignore-engines"); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| await installDependencies({ cwd: projectPath, silent: true, args: installArgs }); | ||
|
Comment on lines
+261
to
+277
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: # First, let's find the installDependencies function and understand the context
fd -t f "installDependencies" --exec rg -l "installDependencies" {} \;Repository: triggerdotdev/trigger.dev Length of output: 51 🏁 Script executed: # Let's check the update.ts file to see imports and context
cat -n packages/cli-v3/src/commands/update.ts | head -50Repository: triggerdotdev/trigger.dev Length of output: 2521 🏁 Script executed: # Search for nypm usage in the codebase
rg "nypm|detectPackageManager" -t ts -t js --max-count=20Repository: triggerdotdev/trigger.dev Length of output: 2226 🏁 Script executed: # Let's see the full context around lines 261-277 in update.ts
sed -n '240,290p' packages/cli-v3/src/commands/update.ts | cat -nRepository: triggerdotdev/trigger.dev Length of output: 1998 🏁 Script executed: # Let's check the test file to see what flags are being verified
cat -n packages/cli-v3/src/commands/update.test.ts | grep -A 10 -B 5 "ignoreEngines\|installArgs\|--no-engine-strict\|engine-strict\|ignore-engines"Repository: triggerdotdev/trigger.dev Length of output: 2256 🏁 Script executed: # Let's find if there are any inline comments or documentation about these flags
rg "no-engine-strict|engine-strict|ignore-engines" packages/cli-v3 -B 2 -A 2Repository: triggerdotdev/trigger.dev Length of output: 3185 🌐 Web query:
💡 Result:
You can also set it persistently via npm config (e.g., in Docs: npm config option Sources 🌐 Web query:
💡 Result:
So Sources: pnpm CLI “Differences vs npm” ( 🌐 Web query:
💡 Result:
Sources: Citations:
🌐 Web query:
💡 Result: In
Source: References [1] https://app.unpkg.com/nypm@0.5.2/files/dist/index.d.mts Yarn Berry does not support The flags for npm and pnpm are correct, but 🤖 Prompt for AI Agents |
||
| } catch (error) { | ||
| installSpinner.stop( | ||
| `Failed to install new package versions${packageManager ? ` with ${packageManager.name}` : "" | ||
| }` | ||
| ); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mocks are prohibited by repo testing guidelines.
This test suite relies heavily on
vi.mock/vi.mocked, which the repository rules disallow. Please refactor to avoid mocks (e.g., integration-style tests with temp dirs/real dependencies, or testcontainers where applicable).As per coding guidelines: **/*.test.{ts,tsx,js,jsx}: Use vitest exclusively for testing and never mock anything - use testcontainers instead; Tests should avoid mocks or stubs and use the helpers from
@internal/testcontainerswhen Redis or Postgres are needed.🤖 Prompt for AI Agents