-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add nub to recognised package managers #14595
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
base: main
Are you sure you want to change the base?
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,7 @@ | ||
| --- | ||
| "@cloudflare/workers-utils": patch | ||
| --- | ||
|
|
||
| Add nub to the list of recognised package managers | ||
|
|
||
| Projects using nub can now be automatically detected by their `nub.lock` file. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| */ | ||
| export interface PackageManager { | ||
| /** The package manager identifier. */ | ||
| type: "npm" | "yarn" | "pnpm" | "bun"; | ||
| type: "npm" | "yarn" | "pnpm" | "bun" | "nub"; | ||
| /** The command used to execute packages (e.g. `npx`, `pnpm`, `bunx`). */ | ||
| npx: string; | ||
| /** The command segments used to download and execute packages (e.g. `["npx"]`, `["pnpm", "dlx"]`). */ | ||
|
|
@@ -52,3 +52,13 @@ export const BunPackageManager = { | |
| dlx: ["bunx"], | ||
| lockFiles: ["bun.lockb", "bun.lock"], | ||
| } as const satisfies PackageManager; | ||
|
|
||
| /** | ||
| * Manage packages using nub. | ||
| */ | ||
| export const NubPackageManager = { | ||
| type: "nub", | ||
| npx: "nubx", | ||
| dlx: ["nubx"], | ||
| lockFiles: ["nub.lock"], | ||
| } as const satisfies PackageManager; | ||
|
Comment on lines
+59
to
+64
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. 🔍 Downstream package manager detection logic does not handle nub The PR adds
This may be intentional if the PR is a first step (adding the constant so consumers can reference it), but if the goal is full nub support, these detection paths need updating. The changeset description says "Projects using nub can now be automatically detected by their Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { existsSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { describe, it } from "vitest"; | ||
| import { | ||
| BunPackageManager, | ||
| NpmPackageManager, | ||
| NubPackageManager, | ||
| PnpmPackageManager, | ||
| YarnPackageManager, | ||
| } from "../src/package-manager"; | ||
| import { runInTempDir, seed } from "../src/test-helpers"; | ||
| import type { PackageManager } from "../src/package-manager"; | ||
|
|
||
| const packageManagers: PackageManager[] = [ | ||
| NpmPackageManager, | ||
| PnpmPackageManager, | ||
| YarnPackageManager, | ||
| BunPackageManager, | ||
| NubPackageManager, | ||
| ]; | ||
|
|
||
| describe("package managers", () => { | ||
| it("describes nub", ({ expect }) => { | ||
| expect(NubPackageManager).toEqual({ | ||
| type: "nub", | ||
| npx: "nubx", | ||
| dlx: ["nubx"], | ||
| lockFiles: ["nub.lock"], | ||
| }); | ||
| }); | ||
|
|
||
| describe("lock file detection", () => { | ||
| runInTempDir(); | ||
|
|
||
| // Detection is lock-file-based: a project is managed by the package | ||
| // manager whose lock file is present, matching how consumers resolve it. | ||
| const findByLockFile = (dir: string) => | ||
| packageManagers.find((pm) => | ||
| pm.lockFiles.some((lockFile) => existsSync(join(dir, lockFile))) | ||
| ); | ||
|
|
||
| it("detects nub from nub.lock", async ({ expect }) => { | ||
| await seed({ "nub.lock": "" }); | ||
| expect(findByLockFile(process.cwd())).toBe(NubPackageManager); | ||
| }); | ||
| }); | ||
| }); |
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.
🟡 Changeset incorrectly classified as patch instead of minor for a new feature
The new package manager support is classified as a
patchrelease (.changeset/nub-package-manager.md:2) instead ofminor, even though it adds a new public export and extends the type union.Impact: The release version number will not correctly signal to consumers that new API surface was added.
REVIEW.md semver classification rule violation
REVIEW.md explicitly states that "new API capabilities or exports" and "behavior changes that add functionality" should be classified as
minor. This PR addsNubPackageManageras a new export from@cloudflare/workers-utils(packages/workers-utils/src/index.ts:153) and extends thePackageManagertype union with"nub"(packages/workers-utils/src/package-manager.ts:7). Both of these are new API capabilities.REVIEW.md also notes: "The description text matters less than the actual change. A changeset described as 'Support X' is adding a new feature (minor)." The changeset description "Add nub to the list of recognised package managers" is clearly adding support for something new.
Was this helpful? React with 👍 or 👎 to provide feedback.