-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(cli): handle catalog: protocol in dependency update check (#3905) #4965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "trigger.dev": patch | ||
| --- | ||
|
|
||
| Prevent CLI crash when @trigger.dev dependencies use bun/pnpm catalog: protocol. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { getTriggerDependencies, getVersionMismatches, type Dependency } from "./update.js"; | ||
|
|
||
| describe("getTriggerDependencies", () => { | ||
| it("skips dependencies using catalog: and workspace: protocols", async () => { | ||
| const packageJson = { | ||
| dependencies: { | ||
| "@trigger.dev/sdk": "catalog:", | ||
| "@trigger.dev/core": "catalog:default", | ||
| "@trigger.dev/react-hooks": "workspace:*", | ||
| lodash: "^4.17.21", | ||
| }, | ||
| devDependencies: { | ||
| "@trigger.dev/build": "catalog:tools", | ||
| "@trigger.dev/schema-to-json": "workspace:^3.0.0", | ||
| "@trigger.dev/companyicons": "^1.0.0", | ||
| }, | ||
| }; | ||
|
|
||
| const deps = await getTriggerDependencies(packageJson, "/fake/project/package.json"); | ||
|
|
||
| expect(deps).toEqual([]); | ||
| }); | ||
|
|
||
| it("includes normal @trigger.dev dependencies", async () => { | ||
| const packageJson = { | ||
| dependencies: { | ||
| "@trigger.dev/sdk": "^3.0.0", | ||
| }, | ||
| devDependencies: { | ||
| "@trigger.dev/core": "~3.0.0", | ||
| }, | ||
| }; | ||
|
|
||
| const deps = await getTriggerDependencies(packageJson, "/fake/project/package.json"); | ||
|
|
||
| expect(deps).toHaveLength(2); | ||
| expect(deps).toContainEqual({ | ||
| type: "dependencies", | ||
| name: "@trigger.dev/sdk", | ||
| version: "^3.0.0", | ||
| }); | ||
| expect(deps).toContainEqual({ | ||
| type: "devDependencies", | ||
| name: "@trigger.dev/core", | ||
| version: "~3.0.0", | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("getVersionMismatches", () => { | ||
| it("does not throw when encountering non-semver strings like catalog: or workspace:", () => { | ||
| const deps: Dependency[] = [ | ||
| { | ||
| type: "dependencies", | ||
| name: "@trigger.dev/sdk", | ||
| version: "catalog:", | ||
| }, | ||
| { | ||
| type: "dependencies", | ||
| name: "@trigger.dev/core", | ||
| version: "catalog:named", | ||
| }, | ||
| { | ||
| type: "devDependencies", | ||
| name: "@trigger.dev/build", | ||
| version: "workspace:*", | ||
| }, | ||
| { | ||
| type: "devDependencies", | ||
| name: "@trigger.dev/react-hooks", | ||
| version: "invalid-semver-string", | ||
| }, | ||
| ]; | ||
|
|
||
| expect(() => getVersionMismatches(deps, "3.0.0")).not.toThrow(); | ||
|
|
||
| const { mismatches, isDowngrade } = getVersionMismatches(deps, "3.0.0"); | ||
| expect(mismatches).toHaveLength(4); | ||
| expect(isDowngrade).toBe(false); | ||
| }); | ||
|
|
||
| it("correctly identifies downgrades when valid semver is newer than target CLI version", () => { | ||
| const deps: Dependency[] = [ | ||
| { | ||
| type: "dependencies", | ||
| name: "@trigger.dev/sdk", | ||
| version: "^4.0.0", | ||
| }, | ||
| ]; | ||
|
|
||
| const { mismatches, isDowngrade } = getVersionMismatches(deps, "3.0.0"); | ||
| expect(mismatches).toHaveLength(1); | ||
| expect(isDowngrade).toBe(true); | ||
| }); | ||
|
|
||
| it("ignores packages matching targetVersion, 0.0.0, or pkg.pr.new", () => { | ||
| const deps: Dependency[] = [ | ||
| { | ||
| type: "dependencies", | ||
| name: "@trigger.dev/sdk", | ||
| version: "3.0.0", | ||
| }, | ||
| { | ||
| type: "dependencies", | ||
| name: "@trigger.dev/core", | ||
| version: "0.0.0-prerelease", | ||
| }, | ||
| { | ||
| type: "devDependencies", | ||
| name: "@trigger.dev/build", | ||
| version: "https://pkg.pr.new/@trigger.dev/build@123", | ||
| }, | ||
| ]; | ||
|
|
||
| const { mismatches, isDowngrade } = getVersionMismatches(deps, "3.0.0"); | ||
| expect(mismatches).toHaveLength(0); | ||
| expect(isDowngrade).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,45 +116,6 @@ export async function updateTriggerPackages( | |
|
|
||
| logger.debug("Resolved trigger deps", { triggerDependencies }); | ||
|
|
||
| function getVersionMismatches( | ||
| deps: Dependency[], | ||
| targetVersion: string | ||
| ): { | ||
| mismatches: Dependency[]; | ||
| isDowngrade: boolean; | ||
| } { | ||
| logger.debug("Checking for version mismatches", { deps, targetVersion }); | ||
|
|
||
| const mismatches: Dependency[] = []; | ||
|
|
||
| for (const dep of deps) { | ||
| if ( | ||
| dep.version === targetVersion || | ||
| dep.version.startsWith("https://pkg.pr.new") || | ||
| dep.version.startsWith("0.0.0") | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| mismatches.push(dep); | ||
| } | ||
|
|
||
| const isDowngrade = mismatches.some((dep) => { | ||
| const depMinVersion = semver.minVersion(dep.version); | ||
|
|
||
| if (!depMinVersion) { | ||
| return false; | ||
| } | ||
|
|
||
| return semver.gt(depMinVersion, targetVersion); | ||
| }); | ||
|
|
||
| return { | ||
| mismatches, | ||
| isDowngrade, | ||
| }; | ||
| } | ||
|
|
||
| const { mismatches, isDowngrade } = getVersionMismatches(triggerDependencies, cliVersion); | ||
|
|
||
| logger.debug("Version mismatches", { mismatches, isDowngrade }); | ||
|
|
@@ -314,13 +275,60 @@ export async function updateTriggerPackages( | |
| return hasOutput; | ||
| } | ||
|
|
||
| type Dependency = { | ||
| export type Dependency = { | ||
| type: "dependencies" | "devDependencies"; | ||
| name: string; | ||
| version: string; | ||
| }; | ||
|
|
||
| async function getTriggerDependencies( | ||
| export function getVersionMismatches( | ||
| deps: Dependency[], | ||
| targetVersion: string | ||
| ): { | ||
| mismatches: Dependency[]; | ||
| isDowngrade: boolean; | ||
| } { | ||
| logger.debug("Checking for version mismatches", { deps, targetVersion }); | ||
|
|
||
| const mismatches: Dependency[] = []; | ||
|
|
||
| for (const dep of deps) { | ||
| if ( | ||
| dep.version === targetVersion || | ||
| dep.version.startsWith("https://pkg.pr.new") || | ||
| dep.version.startsWith("0.0.0") | ||
| ) { | ||
| continue; | ||
| } | ||
|
|
||
| mismatches.push(dep); | ||
| } | ||
|
|
||
| const isDowngrade = mismatches.some((dep) => { | ||
| if (!semver.validRange(dep.version)) { | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| const depMinVersion = semver.minVersion(dep.version); | ||
|
|
||
| if (!depMinVersion) { | ||
| return false; | ||
| } | ||
|
|
||
| return semver.gt(depMinVersion, targetVersion); | ||
| } catch { | ||
| return false; | ||
| } | ||
| }); | ||
|
|
||
| return { | ||
| mismatches, | ||
| isDowngrade, | ||
| }; | ||
| } | ||
|
|
||
| export async function getTriggerDependencies( | ||
| packageJson: PackageJson, | ||
| packageJsonPath: string | ||
| ): Promise<Dependency[]> { | ||
|
|
@@ -332,7 +340,7 @@ async function getTriggerDependencies( | |
| continue; | ||
| } | ||
|
|
||
| if (version.startsWith("workspace")) { | ||
| if (version.startsWith("workspace") || version.startsWith("catalog:")) { | ||
| continue; | ||
| } | ||
|
Comment on lines
+343
to
345
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Catalog dependencies bypass version enforcement With installed catalog dependencies, Learn moreCatalog specifiers such as Example: A workspace catalog pins Recommended fix: Resolve catalog dependencies and retain their protocol metadata separately from the concrete installed version. Use the concrete version for mismatch enforcement. When applying an update, modify the relevant catalog definition or emit actionable guidance instead of replacing the package manifest's catalog reference. Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
|
|
||
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.
🔍 Unknown protocols remain mismatches
getVersionMismatchesavoids the crash but retains every unknown protocol inmismatches. Required checks still abort, while interactive updates replace those specifiers with the CLI version.Was this helpful? React with 👍 or 👎 to provide feedback.