Skip to content

fix(migrate): keep catalog: specs intact through vp up under pnpm - #2425

Open
fengmk2 wants to merge 9 commits into
mainfrom
fix/vp-up-catalog-protocol
Open

fix(migrate): keep catalog: specs intact through vp up under pnpm#2425
fengmk2 wants to merge 9 commits into
mainfrom
fix/vp-up-catalog-protocol

Conversation

@fengmk2

@fengmk2 fengmk2 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Fixes #2309. vp up rewrote "vite": "catalog:" to the concrete npm:@voidzero-dev/vite-plus-core@x alias. The catalog already pins the version, so the update was a no-op that destroyed the reference.

vp up passes through to pnpm update. pnpm applies an override by replacing the declared spec on every manifest, and a bare override key matches any spec, catalog: included.

The fix range-qualifies the managed override keys: vite@* and vitest@*. The range matches the semver ranges that transitive and peer declarations use, and never matches catalog:, which is not a valid range. One vp migrate re-keys existing projects.

Caveats: pnpm does not document this override-key behavior. Verified on pnpm 10.34.4 through 12.0.0-rc.3 (pnpm 12 also fixes the clobber upstream); the snapshot fixtures are the tripwire if pnpm changes it. On pnpm 9 to 11 the ranged key skips a declaration that pins an exact prerelease; the pnpmOverrideKey doc comment records that gap.

Repro: https://github.com/why-reproductions-are-required/vite-plus-up-catalog-protocol-2309

@netlify

netlify Bot commented Aug 12, 2026

Copy link
Copy Markdown

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit ccf26fd
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/6a7f38daed0b84000817bdbc

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

✅ Staging deployment successful!

Preview: https://viteplus-staging.void.app/
Commit: ccf26fd

@fengmk2 fengmk2 self-assigned this Aug 12, 2026
@fengmk2
fengmk2 force-pushed the fix/vp-up-catalog-protocol branch from 19982a8 to 3d8f17b Compare August 12, 2026 05:48
@fengmk2 fengmk2 added the preview-build Publish this PR's commits to the registry bridge as preview builds label Aug 12, 2026
@fengmk2

fengmk2 commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d8f17baf2

ℹ️ 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".

Comment thread packages/cli/src/migration/migrator/orchestrators.ts Outdated
Comment thread packages/cli/src/migration/migrator/vitest-ecosystem.ts Outdated
Comment thread packages/cli/src/migration/migrator/vite-plus-bootstrap.ts Outdated
Comment thread packages/cli/src/migration/migrator/orchestrators.ts Outdated
@fengmk2

fengmk2 commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4321a19ebe

ℹ️ 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".

Comment thread packages/cli/src/migration/migrator/shared.ts
@fengmk2
fengmk2 force-pushed the fix/vp-up-catalog-protocol branch from 47cc4bb to 8b2ce1d Compare August 14, 2026 09:20
@fengmk2

fengmk2 commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 8b2ce1d9e0

ℹ️ 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".

pnpm applies overrides through a read-package hook that replaces the
declared spec on every manifest, importers included, before resolution
runs. A bare override key carries no range, and pnpm treats "no range" as
"matches every declared spec", `catalog:` included. An importer declaring
`vite: "catalog:"` therefore loses its catalog provenance during
resolution, so `pnpm update` writes the resolved core alias back into its
package.json (`vite: "npm:@voidzero-dev/vite-plus-core@0.2.7"`).

Range-qualify the managed pnpm override keys (`vite@*`, `vitest@*`). `*`
is a valid range and intersects every real semver range, which is what
the transitive and peer `vite` declarations the override exists for
always use, while `catalog:` is not a valid range and so never matches.
Nothing changes about what gets installed: an importer referencing the
catalog already resolves to the aliased core through its catalog entry.

Migration re-keys a project still carrying the bare key, preserving its
`catalog:<name>` choice, and pending-detection reads a bare managed key
as unsatisfied so one `vp migrate` repairs an existing project. npm/bun
`overrides` and yarn `resolutions` keep bare keys; they have no
`catalog:` importer specs to lose.

Closes #2309
…sion

The fixture hand-wrote `vite: npm:@voidzero-dev/vite-plus-core@latest` in
the catalog, so `vp up` had a real version bump to perform and the clobber
read as a side effect of that resolution. The reported case pins an exact
version, where the update is a NO-OP and the catalog reference is still
destroyed, which is both the reporter's shape and the sharper repro.

Let `vp migrate` write the pinned version instead of hard-coding one that
would churn each release, then rewind the override key to the pre-fix bare
spelling with `vpt replace-file-content` (which also fails loudly if
migrate ever stops writing the ranged key). `vp up` now reports "Already
up to date" and still rewrites `vite: "catalog:"` to the pinned alias.

Reverts the redaction widening from the previous commit: with an exact pin
no snapshot produces the `@^0.2.8` range form any more.
These 11 fixtures timed out on the local recording pass (slow managed Node/package-manager downloads), so they kept the pre-#2309 bare `vite` override key and failed CI on all three platforms. Every diff is the key rename; no behavior changed.
pnpm 12 fixes the clobber upstream: on 12.0.0-rc.3 a bare override key no longer strips a catalog: importer spec, verified by A/B on identical config (pnpm 11.20.0 rewrites it, 12.0.0-rc.3 does not). The range-qualified key vite-plus writes stays correct there, and still redirects an incompatible transitive edge, so it is not merely a pnpm 9-11 workaround.

The vp up screen is omitted: pnpm 12's update summary reports a package-count delta that churns with the bundled dependency graph.
Apply ASD-STE100 style to the comments and guide text added by this branch: one instruction per sentence, active voice, no ellipsis, and sentences under 25 words. The longest sentence was 52 words. Also removes the em dashes these additions introduced.

Comments and documentation only. No code changed.
Four gaps in the range-qualified override key change.

The monorepo root writer was missed. `rewriteMonorepo` routes root pnpm
settings through `rewriteRootWorkspacePackageJson`, not the standalone
writer, and its pre-10.6.2 branch still spread bare `...managed` keys into
`pkg.pnpm.overrides`. Those workspace-wide bare keys kept matching child
`catalog:` declarations, so `vp up` could still replace them. Both writers
now share one `mergeManagedPnpmOverrides` helper, which is what let the
paths drift apart in the first place.

That helper also carries a legacy bare key's `catalog:<name>` value onto
the ranged key. The package.json paths previously replaced it with the
concrete managed alias, which disconnected the override from the user's
named catalog and contradicted the documented preservation behaviour. A
`file:` managed spec still wins, since force-override mode has no catalog.

`overridesSatisfyVitePlus` read only the ranged key in `pnpm-ranged` mode,
so a map holding BOTH `vite@*` and a legacy bare `vite` reported satisfied.
The fast path then skipped the rewrite and left the clobbering key active.
Any remaining bare managed key now reports unsatisfied.

The managed-vitest sweep was over-broad. `removeManagedVitestEntry` and
`removeYamlMapVitestEntry` deleted `vitest@*` from every sink, but only pnpm
overrides use that spelling. npm also accepts a range in an override key, so
a user-authored `"vitest@*"` in npm `overrides` was silently deleted. Both
now take a key style and sweep the ranged spelling for pnpm sinks only.

Each fix has a unit test that fails without it.
Codex review on 4321a19: `vite@*` does not match a declaration pinning an exact prerelease. pnpm compares with semver.intersects(declaredSpec, keyRange), which is asymmetric for prereleases: intersects('8.0.0-beta.18', '*') is false while the reverse is true. My earlier justification checked the wrong argument order.

Verified on pnpm 11.20.0: a bare key redirects an exact-prerelease declaration, the ranged key does not. No range string avoids it, because node-semver only admits a prerelease when a comparator carries the same version tuple. A prerelease RANGE (^8.0.0-beta.1) still matches, and pnpm 12.0.0-rc.3 matches the exact form too, so the gap is pnpm 9-11 only. A bare key would close it but is what clobbers catalog:, so below pnpm 12 the two cannot both hold.

Corrects the comments and guide text that claimed the range keeps the override on every valid semver range.
The pnpm 9 to 11 exact-prerelease gap stays in the `pnpmOverrideKey`
comment. The migrate-rules and upgrade guides drop it per review.

Apply STE style to the prose added after 98a2fd3: split sentences
over the length limits, keep one statement per sentence, use active
voice, and remove the em dash from the pnpm override bullet.
@fengmk2
fengmk2 force-pushed the fix/vp-up-catalog-protocol branch from 8b2ce1d to 8eb791c Compare August 14, 2026 15:11
Apply the /simplify review findings on this branch's code:

- Drop the `deleteYamlMapKey` helper. Its premise was wrong: yaml 2.9.0's
  `YAMLMap.delete` resolves a plain string key through `findPair`, verified
  against the installed package. The YAML sink now deletes the bare key
  directly, with the same missing-map guard.
- Probe the two managed `vitest` key spellings directly instead of scanning
  every sink entry through a predicate; `managedVitestOverrideKeys` now owns
  the spelling list and `isManagedVitestOverrideKey` is gone.
- Fold the force-override `vite-plus` pin into `mergeManagedPnpmOverrides`,
  so both package.json `pnpm.overrides` writers really do go through the one
  helper and the twin call-site blocks collapse to one line each.
- Restructure `ensureOverrideEntries` to move-then-check: the bare key moves
  onto the ranged key first, which removes the trailing re-materialize branch
  and its cross-branch `changed` invariant.
- Write the lingering-vitest check so the ranged spelling only appears under
  `keyStyle === 'pnpm-ranged'`; the old disjunction read the same key twice
  for bare sinks.
@fengmk2
fengmk2 marked this pull request as ready for review August 14, 2026 15:49
@fengmk2
fengmk2 requested a review from wan9chi August 14, 2026 15:49
@fengmk2

fengmk2 commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: ccf26fd42e

ℹ️ 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".

@fengmk2 fengmk2 added preview-build Publish this PR's commits to the registry bridge as preview builds and removed preview-build Publish this PR's commits to the registry bridge as preview builds labels Aug 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Registry bridge build (ccf26fd)

This commit build is published to the registry bridge, which serves these as ordinary npm versions (every other package proxies to npmjs):

Package Version
vite-plus 0.0.0-commit.ccf26fd42e409003faf2e0031531261c59303851
@voidzero-dev/vite-plus-core 0.0.0-commit.ccf26fd42e409003faf2e0031531261c59303851

Install the Vite+ CLI built from this commit, then migrate a project:

# macOS / Linux
curl -fsSL https://vite.plus | VP_PR_VERSION=2425 bash
# Windows (PowerShell)
$env:VP_PR_VERSION="2425"; irm https://vite.plus/ps1 | iex

After installing, upgrade the current project's vite-plus to this test build with:

vp migrate

Or point your package manager at the bridge registry https://registry-bridge.viteplus.dev/:

Package manager Registry config
npm / pnpm / Bun .npmrc: registry=https://registry-bridge.viteplus.dev/
Yarn (v2+) .yarnrc.yml: npmRegistryServer: "https://registry-bridge.viteplus.dev/"

Then pin the build (vite aliases to vite-plus-core; pnpm can use a catalog, npm an overrides entry):

{
  "devDependencies": {
    "vite-plus": "0.0.0-commit.ccf26fd42e409003faf2e0031531261c59303851",
    "vite": "npm:@voidzero-dev/vite-plus-core@0.0.0-commit.ccf26fd42e409003faf2e0031531261c59303851"
  }
}

@github-actions

Copy link
Copy Markdown
Contributor

🐳 Docker preview image

Built from this PR's registry bridge build:

Image Compressed size
ghcr.io/voidzero-dev/vite-plus:pr-2425 236MB
# remove any stale local copy from a previous run, then pull fresh
docker rmi ghcr.io/voidzero-dev/vite-plus:pr-2425 2>/dev/null; docker pull ghcr.io/voidzero-dev/vite-plus:pr-2425

Quick check:

docker run --rm ghcr.io/voidzero-dev/vite-plus:pr-2425 vp --version

See docs/guide/docker.md for usage.

@fengmk2 fengmk2 added test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: create-e2e Run `vp create` e2e tests test: sfw labels Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview-build Publish this PR's commits to the registry bridge as preview builds test: create-e2e Run `vp create` e2e tests test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: sfw

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vp up in monorepo changes vite dependency in package.json from catalog to exact version

1 participant