vp migrate test#10
Draft
fengmk2 wants to merge 2 commits into
Draft
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the project to use vite-plus as its primary toolchain, updating workspace catalogs, package dependencies, configuration files, and adding environment declarations. Key feedback highlights the risk of using temporary pkg.pr.new URLs for core dependencies, duplication and inconsistency in dependency catalogs, and the fragile use of a global CLI in the pre-commit hook. Additionally, the reviewer advises against hardcoding relative paths to build artifacts in configuration files and warns that removing vite from peer dependencies eliminates crucial compatibility safeguards for consumers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+40
to
+42
| vite: https://pkg.pr.new/voidzero-dev/vite-plus/@voidzero-dev/vite-plus-core@dca39038de6ca46e523bf64dc09bf100b636eea9 | ||
| vitest: 4.1.9 | ||
| vite-plus: https://pkg.pr.new/voidzero-dev/vite-plus@dca39038de6ca46e523bf64dc09bf100b636eea9 |
There was a problem hiding this comment.
Using temporary pkg.pr.new URLs for core dependencies like vite and vite-plus in pnpm-workspace.yaml (and package.json overrides) poses a significant risk to build reproducibility and stability.
These URLs point to temporary commit-specific artifacts that can expire or be removed, which would break dependency installation for other developers and CI/CD pipelines in the future.
If this is for testing/migration purposes, please ensure these are reverted to stable version ranges or official pre-releases before merging this PR.
| @@ -0,0 +1 @@ | |||
| vp staged | |||
There was a problem hiding this comment.
Relying on a globally installed vp CLI in a git hook can break the commit workflow for contributors or GUI clients that do not have vp globally installed in their PATH.
Using pnpm vp staged ensures that the workspace-local version of vite-plus is used, making the hook more robust and portable.
pnpm vp staged
Comment on lines
+125
to
+128
| catalog: | ||
| vite: https://pkg.pr.new/voidzero-dev/vite-plus/@voidzero-dev/vite-plus-core@dca39038de6ca46e523bf64dc09bf100b636eea9 | ||
| vitest: 4.1.9 | ||
| vite-plus: https://pkg.pr.new/voidzero-dev/vite-plus@dca39038de6ca46e523bf64dc09bf100b636eea9 |
There was a problem hiding this comment.
There is a duplication of the vite, vitest, and vite-plus dependency definitions between the named vite-stack catalog (lines 40-42) and the default catalog (lines 126-128).
Furthermore, across the workspace package.json files, there is inconsistency in which catalog is used (e.g., some packages use catalog:vite-stack for vite and catalog: for vite-plus, while others use catalog: for both).
This duplication and inconsistent usage can easily lead to version drift and installation issues if one catalog is updated but the other is missed. Consider consolidating these definitions or ensuring consistent catalog usage across all packages.
| "vue" | ||
| ], | ||
| "jsPlugins": [ | ||
| "../../npm/lint-oxlint/dist/index.mjs", |
There was a problem hiding this comment.
Hardcoding a relative path to another package's build artifact (../../npm/lint-oxlint/dist/index.mjs) is fragile and bypasses the package manager's dependency graph. If the package structure changes or if the package is not built yet, this will fail.
Consider adding the package as a workspace dependency in package.json (e.g., "@vizejs/lint-oxlint": "workspace:*" or similar) and referencing it by its package name, or resolving it dynamically.
| "vite": "^7.3.0 || ^8.0.0" | ||
| "vite-plus": "catalog:" | ||
| }, | ||
| "peerDependencies": {}, |
There was a problem hiding this comment.
Removing vite from peerDependencies without replacing it with vite-plus (or another appropriate peer dependency) removes version compatibility safeguards for consumers of this package. If a consumer installs this plugin in a project with an incompatible version of Vite or Vite Plus, it could lead to unexpected build-time or runtime errors without any package manager warnings.
Consider keeping vite (or adding vite-plus) in peerDependencies with an appropriate version range to ensure compatibility.
Detailed Test ReportCommit: Area Summary
Test InventoryTotal tracked cases: 8025 across 1090 files.
Files
Comment truncated at 64000 characters. Open the workflow run for the full job log. |
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
1 participant
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.
Summary
Change Class
Behavior Reference
Verification Evidence
Risk