Add nub to recognised package managers#14595
Conversation
Add a NubPackageManager descriptor so projects with a nub.lock lock file are detected as nub-managed.
🦋 Changeset detectedLatest commit: b1741c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Per REVIEW.md, changesets target users rather than maintainers and should avoid naming internal constructs. Drops the `NubPackageManager` descriptor reference flagged in review.
Adding one entry to the recognised-package-manager list is additive, same scale as the D1-bindings validation changeset (cloudflare#14530), not a headline feature on the order of cloudflare#14474 (cache options for WorkerEntrypoint exports) — patch fits workers-utils' own convention better than minor.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Widening the shared PackageManager["type"] union to include "nub" left the install helpers in @cloudflare/cli-shared-helpers narrowed to npm/pnpm/yarn/bun, so passing packageManager.type into installWrangler and installPackages failed to type-check (TS2345 at autoconfig src/run.ts:182), breaking the workspace build and every downstream job. Teach installPackages, getWorkspaceInstallRootFlag, and installWrangler about nub, mirroring pnpm's CLI (nub is pnpm-CLI-compatible): --workspace-root at the workspace root and --no-frozen-lockfile on a full install.
@cloudflare/autoconfig
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
Hi @colinhacks. I see that you add Nub support to |
…autoconfig nub.lock The NubPackageManager descriptor was inert. Detect nub in wrangler from its npm_config_user_agent (checked before npm, which nub's UA also contains) and an installed nub binary; detect nub projects in autoconfig via nub.lock, since @netlify/build-info doesn't recognise it.
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
Re: #14499 — Oops, you're right. The descriptor on its own didn't change any behaviour. The two PRs cover different detection paths:
I've now wired it in: wrangler detects nub via |
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ Command suggestions show wrong executable name for nub users (packages/wrangler/src/deployment-bundle/entry.ts:205-213)
The npx-equivalent lookup falls through to the default case for the newly added "nub" agent (sniffUserAgent() at packages/wrangler/src/deployment-bundle/entry.ts:205), so nub users see suggestions using npx instead of nubx.
Impact: Users running nub as their package manager receive incorrect copy-paste command suggestions.
getNpxEquivalent does not handle the new "nub" return value from sniffUserAgent
The PR adds "nub" to the return type of sniffUserAgent() at packages/wrangler/src/package-manager.ts:127-133, but getNpxEquivalent() at packages/wrangler/src/deployment-bundle/entry.ts:204-214 only has cases for "pnpm", "yarn", and "npm". The "nub" value (and also "bun", pre-existing) falls through to default, returning "npx". For nub, the correct equivalent is "nubx" as defined in NubPackageManager.npx at packages/workers-utils/src/package-manager.ts:61.
View 1 additional finding in Devin Review.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…view - packages/cli/packages.ts: drop the duplicate `case "nub"` left by merging main; the file now matches main's version from cloudflare#14499, resolving the no-duplicate-case lint error. - autoconfig: move the nub.lock check into convertDetectedPackageManager, with a TODO to remove it once netlify/build#7124 (nub in @netlify/build-info) is merged and released. - changeset: bump the affected packages minor instead of patch. - tests: cover nub.lock absent and malformed lock file cases in workers-utils, and nub detection via detectFramework in autoconfig.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
All addressed!
Also removed a duplicate |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
NuroDev
added
the
ci:skip-pr-description-validation
|
Codeowners approval required for this PR:
Show detailed file reviewers |
workers-devprod
left a comment
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
Sorry for the turnover here. Nub has now added its own
nub.locklockfile (was previously piggybacking on other lockfiles) so best practices for detection have changed. Thanks!Adds a
NubPackageManagerdescriptor to@cloudflare/workers-utilsso projects managed by nub are recognised.nub is a package manager with a pnpm-compatible CLI. Its lock file is
nub.lock(pnpm-v9 lockfile format). The new descriptor lets lock-file-based detection identify a nub-managed project, alongside the existing npm/pnpm/yarn/bun entries.The change is additive:
NubPackageManagerinpackages/workers-utils/src/package-manager.ts, mirroring thebundescriptor's shape (npx: "nubx",dlx: ["nubx"],lockFiles: ["nub.lock"]), with"nub"added to thePackageManager["type"]union.index.ts.Scoped to
@cloudflare/workers-utilsonly.