Skip to content

Conversation

@fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Nov 28, 2025

No description provided.

@fengmk2 fengmk2 requested a review from Copilot November 28, 2025 08:21
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit 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.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vite-plus-migration4

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 28, 2025

Deploying egg with  Cloudflare Pages  Cloudflare Pages

Latest commit: 32730ce
Status:🚫  Build failed.

View logs

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 28, 2025

Deploying egg-v3 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 32730ce
Status:🚫  Build failed.

View logs

Copilot finished reviewing on behalf of fengmk2 November 28, 2025 08:24
Copy link
Contributor

Copilot AI left a 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 Eggjs monorepo from oxlint/oxfmt tooling to vite+ (a custom tooling suite from VoidZero). The migration consolidates linting, formatting, and testing under a unified vite-based toolchain.

Key Changes:

  • Replaces oxlint/oxfmt with vite+ commands across all packages and scripts
  • Migrates vitest to @voidzero-dev/vite-plus-test package alias
  • Introduces centralized vite.config.ts for format and lint configuration
  • Adds GitHub package registry authentication for @voidzero-dev packages

Reviewed changes

Copilot reviewed 42 out of 43 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
vite.config.ts New centralized configuration file migrating settings from .oxlintrc.json and .oxfmtrc.json
pnpm-workspace.yaml Updates catalog to replace oxlint/oxfmt with vite+ packages; adds peer dependency rules and overrides
.npmrc Configures GitHub package registry authentication for @voidzero-dev scoped packages
package.json (root) Updates scripts to use vite commands; replaces oxlint/oxfmt devDependencies with @voidzero-dev/vite-plus
.oxlintrc.json Removed - configuration migrated to vite.config.ts
.oxfmtrc.json Removed - configuration migrated to vite.config.ts
packages/*/package.json Updates lint/test scripts from oxlint/vitest to vite lint/vite test; removes oxlint devDependencies
plugins/*/package.json Updates lint/test scripts from oxlint/vitest to vite lint/vite test; removes oxlint devDependencies
tools/*/package.json Updates lint/test scripts from oxlint/vitest to vite lint/vite test; removes oxlint devDependencies
examples/*/package.json Updates test scripts from vitest to vite test; updates lint commands in some examples

Comment on lines +246 to +247
vite: 'catalog:'
vitest: 'catalog:'
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The overrides section now forces both vite and vitest to use the catalog versions (which are aliased to @voidzero-dev packages). This is a significant change from the previous rolldown-vite@^7.1.13 override. Ensure that:

  1. All packages that depend on vite/vitest are compatible with the vite+ versions
  2. This doesn't break any existing functionality that relied on the rolldown-vite fork
  3. The change is intentional and has been tested across all workspace packages

This is a critical override that affects the entire monorepo's build and test infrastructure.

Copilot uses AI. Check for mistakes.
.npmrc Outdated
Comment on lines 1 to 2
//npm.pkg.github.com/:_authToken=${GITHUB_TOKEN}
@voidzero-dev:registry=https://npm.pkg.github.com/
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The GitHub token is being referenced via environment variable ${GITHUB_TOKEN}, but there's no validation or fallback if this variable is not set. This could cause silent failures during package installation. Consider:

  1. Documenting this requirement in README or CONTRIBUTING.md
  2. Adding validation in CI/CD pipelines to ensure GITHUB_TOKEN is set
  3. Providing clear error messages when the token is missing

Additionally, ensure this token has appropriate read permissions for the @voidzero-dev scope packages.

Copilot uses AI. Check for mistakes.
Comment on lines +248 to +254
peerDependencyRules:
allowAny:
- vite
- vitest
allowedVersions:
vite: '*'
vitest: '*'
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The peerDependencyRules configuration uses allowAny and allowedVersions: '*' for both vite and vitest. This effectively disables peer dependency version checking, which could lead to compatibility issues. While this may be necessary for the vite+ migration, consider:

  1. Whether this is a temporary workaround or permanent solution
  2. Adding documentation explaining why these broad permissions are needed
  3. Testing thoroughly to ensure no breaking changes occur from version mismatches

This configuration makes the build less strict and could hide incompatibility issues.

Copilot uses AI. Check for mistakes.
Comment on lines 241 to +243
- oxlint-tsgolint
- oxlint
- oxc-minify
- '@voidzero-dev/*'
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The minimumReleaseAgeExclude list still references oxlint-tsgolint, oxlint, and oxc-minify (lines 240-242), but these packages have been removed from the catalog and are no longer used in the project. While adding @voidzero-dev/* to the exclusion list is correct, consider also removing the obsolete oxlint-related entries to keep the configuration clean and up-to-date.

Copilot uses AI. Check for mistakes.
@fengmk2 fengmk2 force-pushed the vite-plus-migration4 branch from b55ec07 to 38adb66 Compare November 28, 2025 08:33
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.58%. Comparing base (fa85b8c) to head (32730ce).

❗ There is a different number of reports uploaded between BASE (fa85b8c) and HEAD (32730ce). Click for more details.

HEAD has 13 uploads less than BASE
Flag BASE (fa85b8c) HEAD (32730ce)
15 2
Additional details and impacted files
@@             Coverage Diff             @@
##             next    #5709       +/-   ##
===========================================
- Coverage   87.41%   70.58%   -16.83%     
===========================================
  Files         561        1      -560     
  Lines       10932       17    -10915     
  Branches     1238        6     -1232     
===========================================
- Hits         9556       12     -9544     
+ Misses       1292        5     -1287     
+ Partials       84        0       -84     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants