-
Notifications
You must be signed in to change notification settings - Fork 124
chore: use vite+ #643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: use vite+ #643
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReplaces Vitest/Oxfmt/Oxlint toolchain with Vite+ across CI, config, scripts, and tests; removes legacy formatter/linter configs; updates workflows and package.json scripts; switches test imports to Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
commit: |
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly overhauls the project's development infrastructure by adopting Vite+ as the primary and unified toolchain. This strategic shift aims to simplify the development experience by centralizing commands for code quality checks, testing, and other build processes under a single interface. The changes ensure a more cohesive and efficient workflow, backed by detailed guidelines for seamless integration and usage. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #643 +/- ##
==========================================
- Coverage 93.82% 92.77% -1.06%
==========================================
Files 14 10 -4
Lines 1474 747 -727
Branches 289 233 -56
==========================================
- Hits 1383 693 -690
+ Misses 85 51 -34
+ Partials 6 3 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request migrates the project's tooling to vite+, a unified toolchain. This involves removing the standalone configurations for oxlint and oxfmt and consolidating them into vite.config.ts. The package.json scripts and dependencies have been updated accordingly to use vite+ commands. Additionally, test files have been updated to import from vite-plus/test, and documentation has been added to CLAUDE.md to explain the new workflow. The changes are mostly correct and align with the goal of adopting vite+. I've found one potential issue in the new vite.config.ts regarding a configuration path that seems incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates the project from individual tooling (vitest, vite, oxlint, oxfmt) to the unified vite-plus toolchain. Vite+ is a modern development toolchain that wraps multiple tools (Vite, Vitest, Oxlint, Oxfmt) behind a single CLI.
Changes:
- Replaced
vitestand standalone oxlint/oxfmt packages withvite-plus - Updated all test imports from
'vitest'to'vite-plus/test' - Migrated configuration from separate .oxlintrc.json and .oxfmtrc.json files into vite.config.ts
- Updated package.json scripts to use
vitecommands instead of individual tools - Modified GitHub Actions workflows to use vite+ and removed Node 18 from test matrix
- Added comprehensive vite+ documentation in CLAUDE.md
Reviewed changes
Copilot reviewed 52 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Changed import from vitest to vite-plus, moved lint/fmt config inline, commented out codspeed plugin, changed pool from 'threads' to 'forks' |
| package.json | Removed vitest/oxlint/oxfmt dependencies, added vite-plus, updated all scripts to use vite commands, added vite override mappings |
| tsconfig.json | Updated exclude pattern from specific vite.config.ts to wildcard *.config.ts |
| test/*.test.ts | Updated 50+ test files to import from 'vite-plus/test' instead of 'vitest' |
| .oxlintrc.json | Removed (configuration moved to vite.config.ts) |
| .oxfmtrc.json | Removed (configuration moved to vite.config.ts) |
| README.md | Added pkg.pr.new badge |
| CLAUDE.md | Added comprehensive vite+ documentation section |
| .github/workflows/*.yml | Updated to use vite commands and setup-vite-plus-action, added old-versions workflow for Node 16/18 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
11-44: Update "Common Commands" and "Testing" sections to align with Vite+ guidance.Lines 11-44 and 69-77 instruct direct use of
pnpmandVitest, but the Vite+ section (lines 86-129) explicitly prohibits this. Update these earlier sections to usevitecommands instead (e.g.,vite install,vite test,vite lint) to prevent conflicting guidance for users.
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 113-114: Update the lint-staged command entries so the formatter
call does not include a trailing directory argument: replace the "vite run fmt
." invocation with "vite fmt --no-error-on-unmatched-pattern" (no dot) so oxfmt
only formats the staged files passed by lint-staged, and similarly consider
removing explicit paths from "vite run lint --fix" (i.e., use "vite lint --fix"
or rely on lint-staged file-paths) to ensure both the lint and fmt commands
operate on the staged files rather than the entire repo.
In `@README.md`:
- Line 11: The README contains a broken pkg.pr.new badge URL
("https://pkg.pr.new/badge/node-modules/urllib") that returns 404; update the
badge reference in README.md by either correcting the badge path to the proper
OWNER/REPO for the package or removing/replacing the badge with a valid
alternative (e.g., Shields.io or the correct pkg.pr.new badge after confirming
the package is registered). Locate the badge string in README.md and replace the
URL in the markdown image/link to a verified working badge URL.
♻️ Duplicate comments (1)
vite.config.ts (1)
28-29: Acknowledge the FIXME for the incorrect extends path.The FIXME comment documents a known issue with the relative path resolution. Once
vite-plusfixes the extends path resolution, update this to use the correct path:./node_modules/@eggjs/oxlint-config/.oxlintrc.json.
🧹 Nitpick comments (2)
vite.config.ts (1)
30-37: All lint categories are set to 'allow', effectively disabling category-level enforcement.Setting every category to
'allow'neutralizes the category defaults. If this is intentional to rely solely on explicit rule overrides below, consider adding a brief inline comment explaining this strategy for future maintainers.package.json (1)
127-134: Bothpnpm.overridesand top-leveloverridesuse@latest, amplifying reproducibility concerns.When
vite-plus-corepublishes a breaking change, builds may fail unexpectedly. Consider pinning these overrides to a specific version range as well, or document whylatestis necessary during active development.
https://github.com/voidzero-dev/setup-vite-plus-action
Summary by CodeRabbit
New Features
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.