vp migrate beta test#2
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the project to use vite-plus and pnpm catalogs, adds a pre-commit hook, and updates configuration files. However, several critical issues were introduced, likely due to an automated migration script. Specifically, the staged property was incorrectly injected into various return objects in vite.config.ts, including a major bug where a string (html) is spread into an object. Additionally, the postinstall script in package.json was renamed to postinstall2, which prevents it from running automatically.
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.
| if (!ctx.server) return { | ||
| staged: { | ||
| "*": "vp check --fix" | ||
| }, | ||
| ...html, | ||
| } | ||
| if (!process.env.REACT_SCAN) return { | ||
| staged: { | ||
| "*": "vp check --fix" | ||
| }, | ||
| ...html, | ||
| } |
There was a problem hiding this comment.
Spreading a string (...html) into an object literal will result in an object of character indices (e.g., { 0: '<', 1: '!', ... }), which is a major bug. Additionally, transformIndexHtml expects a string or { html, tags } return value, not an object containing staged. This appears to be an accidental replacement by a migration script. It should be reverted to simply returning html.
if (!ctx.server) return html;
if (!process.env.REACT_SCAN) return html;| "dev:web-server": "cross-env NODE_ENV=development node scripts/dev-web-server.mjs", | ||
| "run:web-server": "cross-env NODE_ENV=production vp build && cd web && node index.js", | ||
| "postinstall": "cd web && pnpm install", | ||
| "postinstall2": "cd web && pnpm install", |
There was a problem hiding this comment.
Renaming the postinstall script to postinstall2 will prevent it from running automatically after pnpm install. If this was done to temporarily disable it during migration, please ensure it is reverted to postinstall so that the web package dependencies are correctly installed in normal environments.
| "postinstall2": "cd web && pnpm install", | |
| "postinstall": "cd web && pnpm install", |
| return { | ||
| staged: { | ||
| "*": "vp check --fix" | ||
| }, |
There was a problem hiding this comment.
The staged property is not a valid field for a Vite/Rolldown plugin's transform return object. This appears to be an accidental injection by a migration script and should be removed.
| return { | |
| staged: { | |
| "*": "vp check --fix" | |
| }, | |
| return { |
| return { | ||
| staged: { | ||
| "*": "vp check --fix" | ||
| }, |
There was a problem hiding this comment.
The staged property is not a valid field for the returned favicon object. This appears to be an accidental injection by a migration script and should be removed.
| return { | |
| staged: { | |
| "*": "vp check --fix" | |
| }, | |
| return { |
Fixes
Issue Number:
What is the current behavior?
What is the new behavior?
Other information
PR Checklist
Toggle...
What kind of change does this PR introduce?
Please check all that apply to this PR using "x":