Uh oh!
There was an error while loading. Please reload this page.
fix(core): resolve Rolldown bindings via platform packages - #2067
fix(core): resolve Rolldown bindings via platform packages#2067jong-kyung wants to merge 32 commits into
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
jong-kyung
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:33d915dd59
ℹ️ About Codex in GitHub
Your 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".
| } | ||
| }, | ||
| "optionalDependencies": { | ||
| "@voidzero-dev/vite-plus-darwin-arm64": "0.2.2", |
There was a problem hiding this comment.
Keep native optional deps in the version bump
When the release prep workflow bumps versions, it only rewrites the top-level version field in packages/core/package.json (.github/workflows/prepare_release.yml:55-56), leaving these newly added native optional dependency pins at the previous release. The new artifact test also requires each Vite+ native optional dependency to equal corePkgJson.version (packages/core/__tests__/build-artifacts.spec.ts:57), so the next release: v... PR will fail as soon as core is bumped unless the bump path updates these pins too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'm not sure what the preferred fix is for this release-flow issue. I avoided updating these pins in prepare_release.yml because the new native packages are not published yet at that point, so resolving exact versions during release prep seemed unsafe.
@fengmk2 Could you advise which approach you'd prefer?
There was a problem hiding this comment.
It shouldn't be added here. The napi binary dependency is automatically added by napi-rs/cli during the publishing process in packages/cli/publish-native-addons.ts, so it doesn't need to be depended on at the code level. However, currently it doesn't support automatically adding this dependency configuration to 2 packages. Need @Brooooooklyn to help review whether our approach is correct. I have a feeling something is wrong.
Uh oh!
There was an error while loading. Please reload this page.
jong-kyung
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:9a4307d07a
ℹ️ About Codex in GitHub
Your 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jong-kyung
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:ed475572ba
ℹ️ About Codex in GitHub
Your 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jong-kyung
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:cf5b76793b
ℹ️ About Codex in GitHub
Your 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
feff11d to
9868852Comparejong-kyung
commented
Jul 6, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:986885258c
ℹ️ About Codex in GitHub
Your 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".
| }; | ||
| const { code, errors } = await format(destPkgPath, JSON.stringify(destPkg, null, 2) + '\n', { | ||
| destPkg.optionalDependencies = { ...destPkg.optionalDependencies }; |
There was a problem hiding this comment.
Update native deps during version bumps
When the next release PR is created, .github/workflows/prepare_release.yml only bumps the top-level version fields, leaving these newly hard-coded native optional dependency specs at the previous release. The CI build then runs mergePackageJson(), which rewrites them to destPkg.version, so the post-build dirty-tree check in .github/workflows/ci.yml fails; after merge, main would also keep @voidzero-dev/vite-plus-core out of sync with the native packages until the ephemeral publish step patches it. Please update the release-prep path (or stop source builds from rewriting these specs) so the committed package.json stays consistent across version bumps.
Useful? React with 👍 / 👎.
jong-kyung
commented
Jul 7, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:fcfaca56bc
ℹ️ About Codex in GitHub
Your 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".
Uh oh!
There was an error while loading. Please reload this page.
fengmk2
commented
Jul 7, 2026
This change is a bit complex, so I won't merge it in this week's release. I'll review it after today's release is done. |
jong-kyung
commented
Jul 7, 2026
I agree. I’ll review and address your feedback when you take a look later. |
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
Signed-off-by: JongKyung Lee <jongkyung0413@gmail.com>
fengmk2
commented
Aug 3, 2026
@jong-kyung Let me take over this issue, I need to write a RFC first. |
Release-built core resolves bundled Rolldown's native binding through vite-plus/binding, an export of a package core never declares, which breaks pnpm enable-global-virtual-store, Yarn PnP, and standalone core installs. The RFC keeps the per-platform rewrite direction from PR #2067 but moves the platform optionalDependencies pins out of the committed package.json and into the publish pipeline, mirroring how napi-rs injects the same packages into vite-plus.
Release-built core resolves bundled Rolldown's native binding through vite-plus/binding, an export of a package core never declares, which breaks pnpm enable-global-virtual-store, Yarn PnP, and standalone core installs. The RFC keeps the per-platform rewrite direction from PR #2067 but moves the platform optionalDependencies pins out of the committed package.json and into the publish pipeline, mirroring how napi-rs injects the same packages into vite-plus.
Release-built core resolves bundled Rolldown's native binding through vite-plus/binding, an export of a package core never declares, which breaks pnpm enable-global-virtual-store, Yarn PnP, and standalone core installs. The RFC keeps the per-platform rewrite direction from PR #2067 but moves the platform optionalDependencies pins out of the committed package.json and into the publish pipeline, mirroring how napi-rs injects the same packages into vite-plus.
Summary
Fixes#2054.
This updates
@voidzero-dev/vite-plus-corerelease builds so bundled Rolldown no longer resolves native bindings throughvite-plus/binding.Instead, supported Rolldown native package specifiers are rewritten from:
to the matching Vite+ native platform package:
vite-plus/binding remains available as a compatibility export.
Why
With pnpm enable-global-virtual-store, core can run from pnpm's global store realpath. In that layout, core cannot
reliably resolve undeclared vite-plus/binding.
This avoids the circular package graph:
and makes core's native binding resolution use declared optional dependencies instead.
Changes