Repository navigation
fix(migrate): keep bun catalog specs when workspaces.packages is empty - #3397
jong-kyung wants to merge 8 commits into
Conversation
Bun resolves catalog: references whenever the root package.json has a workspaces field, even an empty one. The upgrade path required a non-empty package list, so a single-package Bun project with workspaces.catalog had its catalog: specs rewritten to concrete versions. Fixes voidzero-dev#3375
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jong-kyung
added
test: e2e
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d230f70b81
ℹ️ 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".
Bun before 1.2.21 reads only workspaces.catalog, so a top-level catalog fails to resolve there. Keep concrete specs on those versions unless the project already has a workspaces.catalog, which rewriteBunCatalog keeps in place.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89c9c09115
ℹ️ 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".
The fresh monorepo path always wrote a top-level catalog for Bun, which Bun before 1.2.21 cannot resolve. Share bunSupportsCatalog between the fresh and upgrade paths, keep root overrides concrete when catalogs are unsupported, and start the Bun 1.2.20 snapshot without vite-plus installed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91640ae94e
ℹ️ 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".
A catalog: vite-plus spec always looks current, so a stale catalog value let detectVitePlusBootstrapPending report a Bun workspace as migrated and skip the upgrade. Check the catalog value like the pnpm and Yarn paths do.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26eadcb27c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2cb1963b7
ℹ️ 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".
alignVitestEcosystemPackages turns an alignable @vitest/* dependency into a catalog: reference, but the Bun catalog rewrite only updated entries it already had, so bun install failed with "not in catalog". Add the missing entries when the catalog owns vitest, matching the pnpm catalog.
When a named Bun catalog already owns the toolchain, new references point at it, but rewriteBunCatalog added missing entries only to the default catalog, so vite-plus@catalog:<name> failed to resolve. Add them to that named catalog instead of creating a default catalog beside it, as the pnpm catalog already does, and update the Bun migration rules.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 163939de46
ℹ️ 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".
| // Like the pnpm catalog, add every declared alignable @vitest/* when the | ||
| // catalog owns `vitest`, so its new `catalog:` reference resolves. #2005 | ||
| const names = | ||
| addMissing && 'vitest' in catalog |
There was a problem hiding this comment.
[P2] Add ecosystem catalog entries without requiring a direct Vitest runner
For a Bun project pinned to 1.2.20 with workspaces.packages: [], an existing workspaces.catalog, and a concrete @vitest/utils dependency absent from that catalog, the newly enabled catalog path produces an unresolved dependency. @vitest/utils is deliberately excluded from direct-runner detection, so usesVitest is false and the catalog has no vitest entry. This condition therefore skips the utility entry, although alignVitestEcosystemPackages changes its dependency spec to catalog:.
I reproduced this with ensureVitePlusBootstrap: the current head writes devDependencies["@vitest/utils"] = "catalog:" without creating the matching catalog entry; the base writes the concrete bundled version, 5.1.0. Please add missing alignable ecosystem entries whenever their dependency specs use the selected catalog, independently of whether vitest itself is required, and cover an excluded utility such as @vitest/utils. The analogous pnpm issue does not remove the new regression for empty Bun workspaces.
| bunSupportsCatalog( | ||
| readJsonFile(path.join(workspaceInfo.rootDir, 'package.json')), | ||
| workspaceInfo.downloadPackageManager.version, |
There was a problem hiding this comment.
[P2] Re-pin existing workspace Vite+ dependencies in the concrete fallback
For a Bun 1.2.20 workspace without a nested catalog, this change makes full migration pass supportCatalog = false to package rewriting. If the root has not adopted Vite+ but a child already declares vite-plus: "0.1.20", that child keeps its old version because shouldNormalizeExistingVitePlus in package-json.ts requires catalog support.
I reproduced this with a root declaring workspaces: ["packages/*"], packageManager: "bun@1.2.20", and devDependencies.vite: "^7.0.0", plus a child declaring vite-plus: "0.1.20". After rewriteMonorepo, the root uses Vite+ 1.5.0 and the shared Vite override targets core 1.5.0, but the child still uses 0.1.20. The base normalizes the child through the catalog to the target version; this comparison verifies version normalization, not that the base's catalog layout works on old Bun.
Please normalize existing plain Vite+ versions to canonicalVitePlusSpec even when catalogs are unavailable, while preserving intentional protocol pins. Add coverage for a full migration where the root lacks Vite+ and a child already has it.
|
Thanks for both reviews. I had planned to handle these in a follow-up PR, but I'll widen the scope of this PR so the fixes apply to every package manager, then request another review. |
Fixes #3375.
Bun resolves
catalog:references whenever the rootpackage.jsonhas aworkspacesfield, even an empty one, butvp migraterequired a non-empty package list. Fixing that exposed other cases where migration wrote catalog references Bun cannot resolve, so this PR also fixes them:workspaces. On those versions, migration keeps concrete specs unless the project already has a catalog there. This applies to both fresh and upgrade migrations.vite-plusvalue in a Bun catalog is now reported as pending, so the upgrade is no longer skipped.@vitest/*entries are added to the Bun catalog when their dependencies becomecatalog:references.