-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: move wrangler to peer dependencies #15080
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: next
Are you sure you want to change the base?
fix: move wrangler to peer dependencies #15080
Conversation
🦋 Changeset detectedLatest commit: 0cf8edb 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 |
|
| 📦 Package | 🔒 Before | 🔓 After |
|---|---|---|
| @cloudflare/kv-asset-handler | trusted-with-provenance | none |
| @cloudflare/unenv-preset | trusted-with-provenance | none |
| workerd | trusted-with-provenance | none |
| undici | provenance | none |
| miniflare | trusted-with-provenance | none |
| youch | provenance | none |
| @cloudflare/workerd-darwin-64 | trusted-with-provenance | none |
| @cloudflare/workerd-darwin-arm64 | trusted-with-provenance | none |
| @cloudflare/workerd-linux-64 | trusted-with-provenance | none |
| @cloudflare/workerd-linux-arm64 | trusted-with-provenance | none |
| @cloudflare/workerd-windows-64 | trusted-with-provenance | none |
| wrangler | trusted-with-provenance | none |
6cefff4 to
1f40233
Compare
| "peerDependencies": { | ||
| "astro": "^6.0.0-alpha.0" | ||
| "astro": "^6.0.0-alpha.0", | ||
| "wrangler": "^4.53.0" |
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.
Would it make sense to use a wider semver range here. @ascorbic do you have any recommendation, what would be the minimal version we need?
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.
Yes, keep it wide and let the Vite plugin choose the right version
alexanderniebuhr
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.
@GameRoMan Thanks for this contribution. I agree that we should move wrangler to be a peerDependency. I left some comments for your to consider. :)
ematipico
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.
The changeset needs to change
0c2ec8e to
b211faf
Compare
ematipico
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.
Looks good to me!
@sarah11918, do we need to do something, docs-wise?
sarah11918
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.
@ematipico I assume we're good re: docs here since it doesn't look like anything is required for the user to do! There's already a big Cloudflare docs PR open, so as long as @ascorbic makes sure that anything here is reflected there as needed, I think this can merge fine on its own!
c23005e to
afe2d9a
Compare
Co-authored-by: Alexander Niebuhr <[email protected]>
Co-authored-by: Matt Kane <[email protected]>
7866e91 to
2049eeb
Compare
Changes
Move wrangler to peer dependencies
Testing
Existing tests should pass
Docs
Not necessary