-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[wrangler] Use project's package manager in wrangler setup #12437
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
Conversation
🦋 Changeset detectedLatest commit: f34f3d1 The changes in this PR will be included in the next version bump. 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 |
|
Claude finished @MattieTK's task —— View job Changeset Review
Typo in titleLine 5 contains a typo: "wranger" should be "wrangler" -fix: use project's package manager in wranger autoconfig
+fix: use project's package manager in wrangler autoconfigOther validation checks:
|
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.
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
30855bc to
b3438c0
Compare
dario-piotrowicz
left a comment
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.
Generally looks good to me, thanks @MattieTK 🙂
I just left a few small comments all non blocking
It seems like the tests however need some fixing 🥲
b3438c0 to
35ebd5e
Compare
dario-piotrowicz
left a comment
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.
LGTM, assuming that the CI failures are addressed of course
Thanks @MattieTK! 🫶
wrangler setup now detects the project's package manager from lockfiles and packageManager field instead of defaulting to npm. This fixes failures in pnpm/yarn workspace projects where npm cannot handle workspace: protocol. Leverages existing @netlify/build-info detection from framework analysis.
The AutoConfigDetailsBase type now requires packageManager, but several test files were constructing AutoConfigDetails without it. Also updated vi.mock calls to preserve NpmPackageManager export and fixed a consistent-type-imports lint violation in setup.test.ts.
08dcf96 to
f34f3d1
Compare
When users run
wrangler setupusing a package manager that doesn't match their project (e.g.npx wrangler setupin a pnpm workspace), the command fails because npm cannot handle theworkspace:protocol used by pnpm and yarn workspaces.The existing
getPackageManager()function relies onnpm_config_user_agentto detect which package manager invoked the command. When this doesn't match the project's actual package manager, autoconfig generates incorrect commands and may fail during dependency installation with errors like:npm error code EUNSUPPORTEDPROTOCOLfor workspace: protocolnpm error Cannot read properties of null (reading 'matches')This change detects the project's package manager from lockfiles and
packageManagerfield rather than from how wrangler was invoked.Why only in autoconfig?
This fix is scoped to
wrangler setuprather than changinggetPackageManager()globally because:@netlify/build-infois already a dependency here for framework detection, and itsProjectclass provides robust package manager detection as a side effect ofgetBuildSettings()