fix(core): declare vite-plus as optional peer - #2053
jong-kyung wants to merge 1 commit into
Conversation
Sorry, something went wrong.
✅ Deploy Preview for viteplus-preview canceled.
|
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b51d5ad05
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
| "typescript": "^5.0.0 || ^6.0.0", | ||
| "unplugin-unused": "^0.5.0", | ||
| "unrun": "*", | ||
| "vite-plus": "0.2.2", |
There was a problem hiding this comment.
Keep vite-plus peer in sync on release bumps
When the release workflow bumps versions, .github/workflows/prepare_release.yml only edits the top-level "version" fields, so this new exact peer remains at the previous release (for example core 0.2.3 would still peer on vite-plus@0.2.2). The publish job later checks out that committed package.json and only downloads built dist artifacts before pnpm publish, so the mergePackageJson() rewrite from the build job is not carried into the published package; update the release/version-stamping paths to rewrite this peer too or future releases will publish a stale peer contract.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was a problem hiding this comment.
This is strange, it has become a kind of circular dependency.
Sorry, something went wrong.
There was a problem hiding this comment.
This is strange, it has become a kind of circular dependency.
I agree, this probably isn’t the right fix.
Since vite-plus depends on @voidzero-dev/vite-plus-core, making core peer-depend on vite-plus creates an awkward circular package relationship. I’ll think more about how to fix the pnpm global virtual store issue without introducing that shape.
Sorry, something went wrong.
There was a problem hiding this comment.
@fengmk2 Maybe a cleaner direction is to move the binding resolver into a neutral package, e.g. @voidzero-dev/vite-plus-binding.
Then both packages can depend on it:
vite-plus -> @voidzero-dev/vite-plus-binding
@voidzero-dev/vite-plus-core -> @voidzero-dev/vite-plus-bindingand the rewrite becomes:
@rolldown/binding-* -> @voidzero-dev/vite-plus-bindingThis avoids the circular vite-plus -> core -> vite-plus/binding shape. WDYT?
Sorry, something went wrong.
There was a problem hiding this comment.
I opened a separate issue for discussion: #2054
Sorry, something went wrong.
Summary
Adds vite-plus as an optional peer dependency of @voidzero-dev/vite-plus-core.
Context
This is based on the Discord discussion here:
https://discord.com/channels/1475973262193459293/1523149835950362795
In the reported Windows + pnpm case, vite-plus/binding resolved correctly from the project root and the native
package was installed, but @voidzero-dev/vite-plus-core was executed from pnpm’s global virtual store realpath:
From that location, Node could not resolve vite-plus/binding, causing Rolldown’s native binding load to fail.
Changes