feat: resolve Vite+ version from package.json / catalog#102
Merged
Conversation
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
added
the
needs-bundle-rebuild
Add a `version-file` input that resolves the Vite+ version from the checked-out project, so CI keeps a single source of truth instead of duplicating the version in workflow YAML. Resolves from a package.json `vite-plus` entry (dependencies / devDependencies / optionalDependencies / peerDependencies). A `catalog:` / `catalog:<name>` entry is resolved through the nearest catalog source, walking up from the manifest: pnpm-workspace.yaml (pnpm), .yarnrc.yml (yarn), or a root package.json `catalog`/`catalogs` (bun). version-file can also point directly at pnpm-workspace.yaml or .yarnrc.yml. An explicit `version` takes precedence; otherwise version-file is used; otherwise it falls back to `latest`. Closes #82
- Collapse isPresent/asVersion into one coercion returning string | undefined, dropping a misleading type guard. - Resolve from a directly-targeted package.json's own default catalog when vite-plus is not listed as a dependency (e.g. a bun workspace root), matching how the upward walk already honors it. Share the top-level/workspaces lookup via packageJsonCatalogEntry. - Derive the unsupported-file error from YAML_CATALOG_SOURCES so it lists every accepted file.
The version normalization stripped a leading `v` unconditionally, rewriting a valid dist-tag like `vnext` to `next`. Only strip `v` before a digit so semver like `v0.2.0` is normalized while dist-tags are preserved.
- Reject non-exact resolved versions (semver ranges, npm:/git:/file: aliases) with an actionable error instead of forwarding a value the registry-only installer 404s on. - Parse catalog YAML with the failsafe schema so an unquoted version like 1.10 keeps its trailing zero instead of coercing to the number 1.1. - Make the catalog walk fully lenient: a malformed entry in a nearer source is skipped so a valid ancestor entry is still found, and the redundant existsSync stat is dropped. - Bound the upward walk at the workspace root so a catalog outside the checked-out repo cannot leak in. - Make the leading-v strip case-sensitive so a capitalized dist-tag like V2beta is preserved.
When neither `version` nor `version-file` is set, resolve the version from the project's package.json (dependency or catalog) before falling back to `latest`. Auto-detection is best-effort: a missing manifest, no vite-plus entry, or an unresolvable spec (e.g. a semver range) silently falls back to `latest` and never fails the run.
Adds test-default-version to the workflow: with no version/version-file input, asserts the action auto-detects an exact pin and a catalog entry from package.json, and falls back to latest (without failing) for an unresolvable semver range.
vp >= 0.2 prints `vp v0.2.0` instead of the old `Global: v0.2.0`, so the stale regex made the action's `version` output (and state) resolve to "unknown". Extract parsing into parseInstalledVpVersion, support both formats, and unit-test it so a future format change can't silently regress.
fengmk2
force-pushed
the
feat/version-file
branch
from
ef84aa8 to
c7df2a6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for resolving the Vite+ version from a checked-out project file (including pnpm/yarn/bun catalog sources) so workflows don’t need to duplicate the version string, and improves parsing of vp --version output for the action’s version output.
Changes:
- Introduces
version-fileinput and default auto-detection from the project’spackage.json(with catalog resolution), falling back tolatestwhen auto-detect can’t produce an exact installable version. - Adds robust parsing for the installed global
vpversion across old/newvp --versionoutput formats. - Updates docs, end-to-end workflow coverage, and regenerates
dist/index.mjs.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/version-file.ts | Implements version resolution from package.json and catalog sources with workspace-bounded upward search. |
| src/version-file.test.ts | Adds comprehensive unit tests for version-file and catalog resolution behavior. |
| src/utils.ts | Adds parseInstalledVpVersion helper to parse vp --version output. |
| src/utils.test.ts | Tests the new parseInstalledVpVersion formats and edge cases. |
| src/types.ts | Extends Inputs with optional versionFile. |
| src/inputs.ts | Reads raw version (may be empty) and adds version-file input parsing. |
| src/inputs.test.ts | Updates expectations for raw version default and adds version-file parsing test. |
| src/index.ts | Adds version resolution precedence logic and uses parseInstalledVpVersion for outputs. |
| README.md | Documents version auto-detection and version-file usage; updates inputs table. |
| action.yml | Adds version-file, changes version default to empty string, and updates input descriptions. |
| .github/workflows/test.yml | Adds an end-to-end job asserting default version resolution behavior across scenarios. |
| dist/index.mjs | Regenerates the compiled action bundle with the new logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The workspace-root boundary only stopped the upward catalog search when the manifest was inside GITHUB_WORKSPACE; an absolute working-directory/version-file outside it with a catalog: spec still walked to the filesystem root and could pick up an unrelated /pnpm-workspace.yaml. Stop before ascending out of the workspace (searching only the manifest's own dir when it starts outside). Also drop a stale hard-coded latest version from a CI comment. Addresses PR review comments.
assertInstallableVersion used a blocklist that missed ranges without an operator character (partial versions like `0.2`/`1`, x-ranges like `0.2.x`); auto-detect then forwarded them to the installer instead of falling back to latest. Switch to an allowlist that accepts only an exact major.minor.patch version or a letter-led dist-tag. Addresses a Codex review finding.
pnpm reads only pnpm-workspace.yaml; there is no .yml variant, so listing pnpm-workspace.yml in YAML_CATALOG_SOURCES was dead support that made it inconsistent with the error messages and docs (which only mention .yaml). Remove it so code, messages, and docs agree. Addresses PR review comments.
…esolved Previously an explicit version-file that couldn't be resolved (missing file, no vite-plus entry, unresolvable range/alias, invalid JSON/YAML) failed the run. Wrap it in tryResolveVitePlusVersionFile, which logs a warning and returns undefined so resolution falls through to latest, matching the auto-detect default's resilience. resolveVitePlusVersionFile stays strict for callers/tests; the explicit path warns (louder than auto-detect's debug) since a silent latest fallback would otherwise mask a misconfigured pin.
…nges Auto-detect now consults the lockfile when package.json only declares a range (e.g. ^0.2.0) that can't be installed directly, pinning the exact resolved version instead of falling back to latest. Parses pnpm-lock.yaml, package-lock.json, npm-shrinkwrap.json, yarn.lock (classic + berry), and bun.lock; the binary bun.lockb warns and falls back. Precedence: version > version-file > package.json (exact/catalog) > lockfile > latest. Adds src/lockfile-version.ts with per-format extractors and unit tests, plus lockfile-pnpm/lockfile-npm end-to-end CI scenarios.
Member
Author
|
@codex review |
- Trim a resolved version before the empty check so a whitespace-only catalog entry reports 'not found' instead of a confusing 'Cannot use ""'. - Gate the lockfile fallback on the project declaring a direct vite-plus dependency, so a transitive / other-workspace entry in a shared lockfile isn't mistaken for this project's pin. - Prefer a readable text bun.lock beside a binary bun.lockb (detectLockFile prioritizes .lockb) so transitional repos still resolve. - Resolve pnpm and npm workspace lockfiles from the entry for the selected working-directory (importer key / package node_modules path) rather than the first vite-plus seen.
Member
Author
|
@codex review |
peerDependencies expresses a compatibility range (not the installed version) and optionalDependencies is not where a build toolchain belongs, so neither is a valid source for deciding which global vp to install. Narrow DEP_FIELDS to dependencies and devDependencies.
vite-plus is a dev toolchain, so devDependencies is its canonical home. Order only matters when a project lists it in both fields, where the devDependencies version is the intended toolchain version.
- Auto-detect the lockfile from the project directory up to the workspace root, so a monorepo working-directory subpackage whose lockfile lives at the repo root resolves its locked version instead of falling back to latest. Share isWithin via utils. - Remove a stray NUL byte in a bun.lockb test fixture that made git treat src/lockfile-version.test.ts as binary.
yarn.lock and bun.lock don't map workspaces to resolutions, so returning the first vite-plus entry could install another workspace package's pinned version. Return the version only when the lockfile records a single distinct vite-plus version; fall back to latest when ambiguous (>1).
The suites transitively load utils.ts (imports statSync) and lockfile-version.ts (imports existsSync); mock those bindings too so the mocks don't break as exercised code paths shift.
…porter - Gate the lockfile fallback on the declared spec being a semver range or catalog: reference; skip non-registry specs (file:/git:/workspace:/link:/npm: aliases, owner/repo shorthands) whose lockfile 'version' isn't the npm package. - pnpm: when the selected importer is present but lacks vite-plus, return undefined instead of falling through to a global packages scan that could leak another workspace's version.
For a non-empty subPath with no per-package (non-hoisted) entry, only fall back to the root node_modules/vite-plus version when packages[subPath] is a lockfile member that declares vite-plus. A nested project absent from the root lockfile now falls back to latest instead of installing the root/other-workspace version.
The importer-less pnpm fallback scan allowed a '/' before vite-plus@, so it matched scoped look-alikes like @acme/vite-plus@9.9.9. Anchor to the start of a package key (indent + optional leading slash) so only the unscoped vite-plus key matches.
- isWithin / workspaceSubPath: a leading '..' now only counts as escaping the parent at a path-segment boundary, so a child directory named '..foo' isn't misclassified as outside (which could stop the upward catalog/lockfile walk early). workspaceSubPath reuses isWithin. - Clarify the precedence docstring: an unresolvable explicit version-file falls back to 'latest' (it does not continue to auto-detect / lockfile). - Add isWithin unit tests and an owner/repo shorthand gating test; switch the version/lockfile fs mocks to vi.importActual, matching utils.test.ts.
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
version-fileinput to resolve the Vite+ version from the checked-out project, so CI keeps a single source of truth instead of duplicating the version in workflow YAML.Resolution
Reads the
vite-plusentry from apackage.json(dependencies / devDependencies). Acatalog:/catalog:<name>entry is resolved through the nearest catalog source, walking up from the manifest:pnpm-workspace.yaml.yarnrc.ymlpackage.jsoncatalog/catalogs(top-level or underworkspaces)version-filecan also point directly atpnpm-workspace.yamlor.yarnrc.yml. An explicitversiontakes precedence; otherwiseversion-fileis used; otherwise it falls back tolatest. The resolved value must be an exact version or dist-tag (semver ranges are not resolved).Closes #82