Conversation
- Merge @eggjs/tsconfig repository into packages/tsconfig/ - Refactor package to use vitest testing framework instead of Node.js test runner - Update all consuming packages to use workspace:* dependency references - Add vitest configuration with coverage reporting - Convert test assertions from Node.js assert to vitest expect API - Update package.json scripts for vitest and oxlint - Remove external @eggjs/tsconfig catalog dependency in favor of workspace package 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds a new @eggjs/tsconfig package (configs, tests, fixtures, docs, license, .gitignore) and switches multiple packages to reference it via workspace. Removes several package.json homepage fields, updates a bugs URL, and adjusts root/tsconfig build defaults to extend the new tsconfig. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant V as Vitest (test run)
participant C as coffee (child process runner)
participant T as tsc (TypeScript CLI)
participant FS as File System
V->>C: fork tsc -p fixtures/apps/ts-proj/tsconfig.json (cwd=fixture)
C->>T: execute tsc CLI
T->>FS: read tsconfig.json and source files
T->>FS: emit compiled output to dist/
T-->>C: exit code 0
C-->>V: propagate exit code
V->>FS: stat dist/ and assert it's a directory
V-->>V: test passes (exit 0)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @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 refactors the @eggjs/tsconfig package by integrating it into the monorepo and modernizing its development setup. The core purpose is to streamline dependency management by converting @eggjs/tsconfig into a local workspace package, thereby improving consistency and development workflow for TypeScript configurations across the project. This change also updates the testing framework to Vitest, ensuring a more robust and modern testing environment for the TypeScript configurations.
Highlights
- Monorepo Integration: The
@eggjs/tsconfigpackage has been integrated directly into the monorepo, moving from an external catalog dependency to a local workspace package underpackages/tsconfig/. - Testing Framework Migration: The
@eggjs/tsconfigpackage's testing infrastructure has been migrated from the Node.js test runner to Vitest, including updated test scripts, configurations, and conversion of test assertions. - Dependency Updates: All consuming packages within the monorepo have been updated to reference
@eggjs/tsconfigusingworkspace:*instead ofcatalog:, streamlining dependency management. - TypeScript Configuration Modernization: A new base
tsconfig.jsonhas been added for@eggjs/tsconfig, targeting ES2024, NodeNext module resolution, and including experimental decorator support, along with other strict compiler options. - Project Metadata Refinement: Various
package.jsonfiles across the monorepo have had theirhomepagefields removed for consistency and to centralize repository information.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Deploying egg with
|
| Latest commit: |
33ce65c
|
| Status: | 🚫 Build failed. |
Deploying egg-v3 with
|
| Latest commit: |
33ce65c
|
| Status: | 🚫 Build failed. |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5460 +/- ##
==========================================
- Coverage 75.90% 75.86% -0.04%
==========================================
Files 94 94
Lines 5553 5553
Branches 1177 1177
==========================================
- Hits 4215 4213 -2
- Misses 1168 1170 +2
Partials 170 170 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tsconfig/CHANGELOG.md (1)
113-115: Remove generator artifact from changelogStray “fatal: No names found…” leaks into user-facing docs.
- * [[`05c0e95`](http://github.com/eggjs/tsconfig/commit/05c0e954eea00398ed63d6449febbc86051c7fb5)] - first impl (#1) (killa <<killa123@126.com>>),fatal: No names found, cannot describe anything. + * [[`05c0e95`](http://github.com/eggjs/tsconfig/commit/05c0e954eea00398ed63d6449febbc86051c7fb5)] - first impl (#1) (killa <<killa123@126.com>>)
🧹 Nitpick comments (9)
pnpm-workspace.yaml (1)
24-24: Optional: consider dropping @eggjs/tsconfig from catalog now that it’s internal.
Having both a workspace:* consumer and a catalog entry can confuse resolution if any package forgets to use workspace:*. If you keep it for external template/reference, add a comment explaining why.Apply if you opt to remove:
- '@eggjs/tsconfig': ^3.1.0packages/tsconfig/README.md (1)
24-33: Use jsonc code fence to avoid invalid JSON comments.
The snippet contains comments; mark as jsonc to prevent confusion.-```json +```jsonc // tsconfig.json { "extends": "@eggjs/tsconfig", // custom config "compilerOptions": { // override @eggjs/tsconfig options here } }packages/tsconfig/test/fixtures/apps/ts-proj/FooDecorator.ts (1)
1-5: Type the decorator precisely using ClassDecorator.Avoid
anyand make the intent explicit.-export default function FooDecorator() { - return function (target: any) { +export default function FooDecorator(): ClassDecorator { + return function (target) { console.log('decorator to class: ', target); }; }packages/tsconfig/tsconfig.json (6)
29-31: Prefer external source maps for smaller artifacts.Inline maps bloat outputs; external maps work well with modern tooling.
- "inlineSourceMap": true, - "declaration": true, + "sourceMap": true, + "declaration": true,
27-28: Redundant lib checks.
skipLibCheckalready covers default libs; you can dropskipDefaultLibCheck.- "skipLibCheck": true, - "skipDefaultLibCheck": true, + "skipLibCheck": true,
7-11: Interop comment vs verbatimModuleSyntax.With
"verbatimModuleSyntax": true, TS won’t synthesize CJS interop; default-importing CJS (e.g.,import assert from 'assert') can break at runtime under NodeNext. If that usage is required, consider removingverbatimModuleSyntaxor refactor imports to ESM-friendly forms (e.g.,import * as assert from 'node:assert'or named ESM exports).Also applies to: 39-41
8-9: Lock Node server typing explicitly.To avoid accidental DOM globals and clarify environment, consider adding
liband Node types."module": "NodeNext", "moduleResolution": "NodeNext", + "lib": ["ES2024"], + "types": ["node"],
2-2: Move editor-onlycompileOnSaveto local config.Keeps the published base config lean and avoids editor coupling.
- "compileOnSave": true, + // moved to developer/editor settings if needed
1-43: Repo guideline vs published base config: clarify split.Guidelines require packages/*/tsconfig.json to extend the root and set
baseUrl: "./". This file is a published base config; extending the repo root here would break consumers. Suggest splitting:
- keep this file as the exported preset,
- add packages/tsconfig/tsconfig.build.json for local builds/tests:
{ "extends": "../../tsconfig.json", "compilerOptions": { "baseUrl": "./" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/tsconfig/test/fixtures/apps/ts-proj/node_modules/@eggjs/tsconfigis excluded by!**/node_modules/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
examples/helloworld-typescript/package.json(1 hunks)package.json(1 hunks)packages/cluster/package.json(2 hunks)packages/core/package.json(1 hunks)packages/egg/package.json(0 hunks)packages/extend2/package.json(1 hunks)packages/mock/package.json(1 hunks)packages/tsconfig/.gitignore(1 hunks)packages/tsconfig/CHANGELOG.md(1 hunks)packages/tsconfig/LICENSE(1 hunks)packages/tsconfig/README.md(1 hunks)packages/tsconfig/package.json(1 hunks)packages/tsconfig/test/fixtures/apps/ts-proj/Foo.ts(1 hunks)packages/tsconfig/test/fixtures/apps/ts-proj/FooDecorator.ts(1 hunks)packages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.json(1 hunks)packages/tsconfig/test/index.test.ts(1 hunks)packages/tsconfig/tsconfig.json(1 hunks)packages/tsconfig/vitest.config.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- packages/egg/package.json
🧰 Additional context used
📓 Path-based instructions (5)
packages/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/package.json: Use workspace:* for internal package dependencies in package.json
Reference external dependencies via pnpm catalog using "catalog:" in package.json
Files:
packages/core/package.jsonpackages/extend2/package.jsonpackages/tsconfig/package.jsonpackages/cluster/package.jsonpackages/mock/package.json
packages/*/vitest.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each package must include a vitest.config.ts for test configuration
Files:
packages/tsconfig/vitest.config.ts
pnpm-workspace.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Add and manage external dependency versions in the catalog section of pnpm-workspace.yaml
Files:
pnpm-workspace.yaml
packages/*/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/test/**/*.test.ts: All packages must use Vitest as the test runner; tests live under test//*.test.ts
Test files should import test functions from vitest (e.g., import { describe, it } from 'vitest')
Use Node.js built-in assert module for assertions in tests
Test files must follow the naming/location pattern test//*.test.ts
Files:
packages/tsconfig/test/index.test.ts
packages/*/tsconfig.json
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/tsconfig.json: All sub-project tsconfig.json files must extend the root config using "extends": "../../tsconfig.json"
Each package tsconfig.json must include compilerOptions.baseUrl set to "./"
Files:
packages/tsconfig/tsconfig.json
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/package.json : Use workspace:* for internal package dependencies in package.json
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to pnpm-workspace.yaml : Add and manage external dependency versions in the catalog section of pnpm-workspace.yaml
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/package.json : Reference external dependencies via pnpm catalog using "catalog:" in package.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/package.json : Use workspace:* for internal package dependencies in package.json
Applied to files:
packages/core/package.jsonpackages/extend2/package.jsonpackages/cluster/package.jsonpackages/mock/package.jsonpackage.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/vitest.config.ts : Each package must include a vitest.config.ts for test configuration
Applied to files:
packages/tsconfig/vitest.config.tspackages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.jsonpackages/tsconfig/test/index.test.tspackage.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/test/**/*.test.ts : Test files should import test functions from vitest (e.g., import { describe, it } from 'vitest')
Applied to files:
packages/tsconfig/vitest.config.tspackages/tsconfig/test/index.test.ts
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/test/**/*.test.ts : All packages must use Vitest as the test runner; tests live under test/**/*.test.ts
Applied to files:
packages/tsconfig/vitest.config.tspackages/tsconfig/test/index.test.tspackage.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/test/**/*.test.ts : Test files must follow the naming/location pattern test/**/*.test.ts
Applied to files:
packages/tsconfig/vitest.config.tspackages/tsconfig/test/index.test.ts
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to pnpm-workspace.yaml : Add and manage external dependency versions in the catalog section of pnpm-workspace.yaml
Applied to files:
pnpm-workspace.yamlpackage.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/egg/src/config/plugin.ts : Configure built-in plugins in packages/egg/src/config/plugin.ts
Applied to files:
pnpm-workspace.yamlpackages/tsconfig/package.jsonpackages/mock/package.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files must extend the root config using "extends": "../../tsconfig.json"
Applied to files:
packages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.jsonpackages/tsconfig/README.mdpackages/extend2/package.jsonpackages/tsconfig/tsconfig.jsonpackages/tsconfig/package.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsconfig.json : Each package tsconfig.json must include compilerOptions.baseUrl set to "./"
Applied to files:
packages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsdown.config.ts : Each package should include tsdown.config.ts for unbundled ESM builds
Applied to files:
packages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.jsonpackages/extend2/package.jsonpackages/tsconfig/tsconfig.jsonpackages/tsconfig/package.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/egg/src/config/config.default.ts : Define default framework configuration in packages/egg/src/config/config.default.ts
Applied to files:
packages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.jsonpackages/tsconfig/README.mdpackages/extend2/package.jsonpackages/tsconfig/package.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/test/**/*.test.ts : Use Node.js built-in assert module for assertions in tests
Applied to files:
packages/tsconfig/test/index.test.ts
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/package.json : Reference external dependencies via pnpm catalog using "catalog:" in package.json
Applied to files:
package.json
🧬 Code graph analysis (2)
packages/tsconfig/test/fixtures/apps/ts-proj/Foo.ts (1)
packages/tsconfig/test/fixtures/apps/ts-proj/FooDecorator.ts (1)
FooDecorator(1-5)
packages/tsconfig/test/fixtures/apps/ts-proj/FooDecorator.ts (1)
packages/tsconfig/test/fixtures/apps/ts-proj/Foo.ts (1)
FooDecorator(3-13)
🪛 Biome (2.1.2)
packages/tsconfig/test/fixtures/apps/ts-proj/Foo.ts
[error] 7-11: This code is unreachable
(lint/correctness/noUnreachable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test / Test (windows-latest, 24)
- GitHub Check: Test / Test (windows-latest, 22)
🔇 Additional comments (13)
packages/mock/package.json (1)
120-120: Approve — switch to workspace: for @eggjs/tsconfig*
No remaining 'catalog:' references were found.packages/core/package.json (1)
65-65: Good: devDependency now uses workspace: for @eggjs/tsconfig.*
Matches the coding guideline for internal deps.package.json (1)
11-11: Root devDependency updated to workspace: — LGTM.*
Keeps tooling consistent across the workspace.examples/helloworld-typescript/package.json (1)
15-15: Example now points to workspace: — looks good.*
Keeps example aligned with monorepo’s local tsconfig package.packages/extend2/package.json (1)
34-34: DevDependency switch to workspace: is correct.*
Consistent with internal dependency policy.packages/tsconfig/LICENSE (1)
1-22: MIT license included — all good.
Matches package metadata and standard text.packages/tsconfig/package.json (1)
33-38: Good use of catalog protocol for externalsdevDependencies correctly use "catalog:"; no internal deps here.
packages/cluster/package.json (2)
41-43: Bugs URL correction looks rightPoints to the monorepo issues page.
58-61: Fix invalid workspace protocol usage"workspace:" without a range won't resolve — use "workspace:*" for internal workspace dependencies.
File: packages/cluster/package.json (lines 58-61)
- "@eggjs/supertest": "workspace:", + "@eggjs/supertest": "workspace:*",Scan returned no matches; re-run to confirm no other occurrences:
rg -nP '"@eggjs/[^"]+"\s*:\s*"workspace:"' packages -g '!**/pnpm-lock.yaml'packages/tsconfig/.gitignore (1)
108-109: Nice fixture exceptionsKeeping fixture node_modules while ignoring their dist is the right balance.
packages/tsconfig/test/fixtures/apps/ts-proj/tsconfig.json (1)
1-7: Verify import extension policyFixture imports a sibling with “.ts”. With NodeNext, prefer “.js” in source or enable allowImportingTsExtensions.
Option A (preferred): update import in Foo.ts (see separate comment).
Option B: enable in fixture:"compilerOptions": { "baseUrl": "./", - "outDir": "dist" + "outDir": "dist", + "allowImportingTsExtensions": true }packages/tsconfig/tsconfig.json (2)
32-35: Decorator metadata enabled globally — no top-level 'reflect-metadata' import foundpnpm-lock.yaml contains reflect-metadata@0.1.14 only as a transitive dep; no import/require of 'reflect-metadata' found in the repo.
- Either remove emitDecoratorMetadata from the shared base tsconfig and enable it only in packages that require runtime metadata.
- Or add reflect-metadata as a direct dependency and import 'reflect-metadata' early in consumer entrypoints so metadata APIs are available.
Location: packages/tsconfig/tsconfig.json (emitDecoratorMetadata / experimentalDecorators).
38-41: Require TypeScript >= 5.8 for these tsconfig options.packages/tsconfig/tsconfig.json (lines 38–41): erasableSyntaxOnly → TS ≥5.8; rewriteRelativeImportExtensions → TS ≥5.7; allowImportingTsExtensions → TS ≥5.0. Workspace’s pnpm-workspace.yaml maps "typescript: '5'" but does not guarantee a ≥5.8 minor—verify/update the resolved TypeScript in package.json/lockfile/CI/editor to >=5.8 or remove/guard the newer options.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/supertest/test/supertest.test.ts (1)
143-162: Close HTTPS server to avoid resource leaksThe HTTPS server created via
https.createServerisn’t explicitly closed. Add aserver.close()(ideally in a finally) to avoid handle leaks under parallel test runs.const server = https.createServer( { key: fs.readFileSync(path.join(fixtures, 'test_key.pem')), cert: fs.readFileSync(path.join(fixtures, 'test_cert.pem')), }, app ); app.get('/', function (_req, res) { res.send('hey'); }); - const res = await request(server).get('/'); - - expect(res.status).toBe(200); - expect(res.text).toBe('hey'); + try { + const res = await request(server).get('/'); + expect(res.status).toBe(200); + expect(res.text).toBe('hey'); + } finally { + server.close(); + }tsconfig.json (1)
1-31: Add ./packages/tsconfig to root tsconfig referencespackages/tsconfig/tsconfig.json exists but is not listed in the root tsconfig "references" — add it or confirm intentional exclusion.
"references": [ { "path": "./packages/egg" }, + { + "path": "./packages/tsconfig" + }, { "path": "./packages/core" }, { "path": "./packages/utils" },
🧹 Nitpick comments (5)
site/docs/tutorials/typescript.md (2)
499-499: Version spec nit: prefer caret range for docs snippetUsing a major-only string is fine, but
^3is more conventional in docs and avoids accidental pinning to 3.0.0 if a user copies this literally.- "@eggjs/tsconfig": "3" + "@eggjs/tsconfig": "^3"
496-504: Monorepo note suggestionSince the repo now provides @eggjs/tsconfig as a workspace package, consider adding a one-line note below this snippet: “Inside the egg monorepo use workspace:* instead of a semver range.” This prevents confusion between external usage vs. in-repo usage.
site/docs/tutorials/typescript.zh-CN.md (2)
496-496: 版本范围微调：文档示例建议使用插入号示例里用主版本字符串可用，但更常见写法是
^3，避免用户复制后意外固定到 3.0.0。- "@eggjs/tsconfig": "3" + "@eggjs/tsconfig": "^3"
493-501: 补充 monorepo 说明建议在代码块下方补充一句说明：在本仓库内部请使用
workspace:*，对外项目再使用^3。可避免读者混淆。tsconfig.json (1)
4-4: Confirm intentional override of targetBase @eggjs/tsconfig likely sets a modern target. Overriding to ES2022 may downlevel features across the monorepo. If not strictly required, consider inheriting the base target by removing this override.
- "target": "ES2022" + // inherit "target" from @eggjs/tsconfig unless there is a concrete need + // "target": "ES2022"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/core/test/fixtures/helloworld-ts/package.json(1 hunks)packages/supertest/test/supertest.test.ts(1 hunks)pnpm-workspace.yaml(1 hunks)site/docs/tutorials/typescript.md(1 hunks)site/docs/tutorials/typescript.zh-CN.md(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/core/test/fixtures/helloworld-ts/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- pnpm-workspace.yaml
🧰 Additional context used
📓 Path-based instructions (2)
tsconfig.json
📄 CodeRabbit inference engine (CLAUDE.md)
Root tsconfig.json must include all packages in the references array
Files:
tsconfig.json
packages/*/test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/*/test/**/*.test.ts: All packages must use Vitest as the test runner; tests live under test//*.test.ts
Test files should import test functions from vitest (e.g., import { describe, it } from 'vitest')
Use Node.js built-in assert module for assertions in tests
Test files must follow the naming/location pattern test//*.test.ts
Files:
packages/supertest/test/supertest.test.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/egg/src/config/config.default.ts : Define default framework configuration in packages/egg/src/config/config.default.ts
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/egg/src/config/plugin.ts : Configure built-in plugins in packages/egg/src/config/plugin.ts
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files must extend the root config using "extends": "../../tsconfig.json"
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsdown.config.ts : Each package should include tsdown.config.ts for unbundled ESM builds
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/egg/src/config/config.default.ts : Define default framework configuration in packages/egg/src/config/config.default.ts
Applied to files:
tsconfig.jsonsite/docs/tutorials/typescript.mdsite/docs/tutorials/typescript.zh-CN.md
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files must extend the root config using "extends": "../../tsconfig.json"
Applied to files:
tsconfig.json
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/egg/src/config/plugin.ts : Configure built-in plugins in packages/egg/src/config/plugin.ts
Applied to files:
tsconfig.jsonsite/docs/tutorials/typescript.mdsite/docs/tutorials/typescript.zh-CN.md
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/tsdown.config.ts : Each package should include tsdown.config.ts for unbundled ESM builds
Applied to files:
site/docs/tutorials/typescript.mdsite/docs/tutorials/typescript.zh-CN.md
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/test/**/*.test.ts : Test files should import test functions from vitest (e.g., import { describe, it } from 'vitest')
Applied to files:
packages/supertest/test/supertest.test.ts
📚 Learning: 2025-09-13T13:03:58.196Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T13:03:58.196Z
Learning: Applies to packages/*/test/**/*.test.ts : Use Node.js built-in assert module for assertions in tests
Applied to files:
packages/supertest/test/supertest.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test / Test (windows-latest, 22)
- GitHub Check: Test / Test (windows-latest, 24)
🔇 Additional comments (3)
packages/supertest/test/supertest.test.ts (3)
5-5: Type-only import is correctUsing a type-only import for AddressInfo avoids unnecessary runtime bindings. LGTM.
8-8: Express type-only import is appropriateKeeping the runtime default import for express and type-only import for Express is the right split. LGTM.
12-13: Clarify assertion strategy: Vitest expect vs Node assertRepo guidelines say to use vitest for test runner but Node’s assert for assertions, whereas this file mixes both and predominantly uses
expect. Please confirm the intended standard:
- If moving to
expectis the new norm, update the guidelines accordingly.- If
assertis required, we’ll need to refactor tests to dropexpect.
1 participant
🤖 Generated with Claude Code
Summary by CodeRabbit