refactor: remove egg-ts-helper #5657
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. WalkthroughThis PR removes egg-ts-helper integration (workspace/dependency entries, postinstall hook, auto-run logic), strips egg-ts-helper guidance from TypeScript docs, deletes related tests/fixtures/typings, and updates several egg-bin debug namespace strings and the declarations flag description. Changes
Sequence Diagram(s)sequenceDiagram
participant DevPkg as Developer package
participant Postinst as postinstall.mjs (was)
participant ETS as egg-ts-helper (was)
participant EggBin as egg-bin CLI
rect rgb(230,245,255)
Note over DevPkg,Postinst: OLD flow (before this PR)
DevPkg->>Postinst: install triggers postinstall
Postinst->>DevPkg: read package.json (egg.declarations)
alt declarations enabled & framework present
Postinst->>ETS: spawn ETS bin in project context
ETS-->>EggBin: typings/registration actions
end
end
rect rgb(245,255,230)
Note over DevPkg,EggBin: NEW flow (after this PR)
DevPkg--xPostinst: no postinstall (file removed)
DevPkg->>EggBin: install/run without ETS auto-run
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
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 |
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 focuses on a significant refactoring effort to remove the 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
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes the deprecated egg-ts-helper dependency and all related functionality from the Eggjs framework. The egg-ts-helper was previously used to automatically generate TypeScript declaration files for Egg's auto-loaded modules, but is being removed as part of modernizing the framework.
Key Changes:
- Removed
egg-ts-helperdependency from package catalogs and build configurations - Deleted the postinstall script that ran
egg-ts-helperautomatically - Removed the
--declarationsflag functionality from CLI commands - Updated documentation to remove references to
egg-ts-helper
Reviewed Changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/egg-bin/test/postinstall.test.ts | Deleted entire test file for postinstall script functionality |
| tools/egg-bin/test/fixtures/test-postinstall/package.json | Deleted test fixture package.json used for postinstall tests |
| tools/egg-bin/test/commands/dev.test.ts | Removed skipped test case that verified egg-ts-helper integration with dev command |
| tools/egg-bin/src/commands/test.ts | Updated debug namespace from egg-bin/commands/test to egg/bin/commands/test |
| tools/egg-bin/src/commands/dev.ts | Updated debug namespace from egg-bin/commands/dev to egg/bin/commands/dev |
| tools/egg-bin/src/baseCommand.ts | Removed egg-ts-helper execution logic and deprecated the --declarations flag |
| tools/egg-bin/scripts/start-cluster.mjs | Updated debug namespace from egg-bin/scripts/start-cluster to egg/bin/scripts/start-cluster |
| tools/egg-bin/scripts/start-cluster.cjs | Updated debug namespace and removed unused eslint-disable comment |
| tools/egg-bin/scripts/postinstall.mjs | Deleted entire postinstall script that ran egg-ts-helper |
| tools/egg-bin/package.json | Removed egg-ts-helper dependency and postinstall script hook |
| site/docs/zh-CN/tutorials/typescript.md | Removed Chinese documentation sections about egg-ts-helper usage |
| site/docs/tutorials/typescript.md | Removed English documentation sections about egg-ts-helper usage and fixed frontmatter formatting |
| pnpm-workspace.yaml | Removed egg-ts-helper from catalog dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| --- | ||
| title: TypeScript | ||
| --- | ||
| # TypeScript |
There was a problem hiding this comment.
The frontmatter YAML block (---) was removed, which may break documentation site rendering. The title metadata is typically required by documentation generators like Dumi. Consider restoring the frontmatter block or verify that the documentation site can properly render markdown files without it.
| If the egg's plugin has the right declaration, we need to import it exclipitly and ts can load the related object. | ||
|
|
||
| If you use `egg-ts-helper`, it will automatically generate the exclipit importing declarations according to what plugins you've enabled in the application. If you don't use that, you have to import the declaration of plugins manually. | ||
| If you don't use that, you have to import the declaration of plugins manually. |
There was a problem hiding this comment.
The sentence fragment 'If you don't use that' is unclear after removing the egg-ts-helper context. The preceding sentence should be updated to clarify what 'that' refers to, or this sentence should be revised to stand alone without the removed context.
| If you don't use that, you have to import the declaration of plugins manually. | |
| If you do not import the plugin declaration explicitly, you have to import the declaration of plugins manually. |
Deploying egg with
|
| Latest commit: |
801802f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e7dc774b.egg-cci.pages.dev |
| Branch Preview URL: | https://dont-run-ets.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5657 +/- ##
==========================================
- Coverage 87.68% 87.58% -0.11%
==========================================
Files 565 561 -4
Lines 11010 10911 -99
Branches 1243 1231 -12
==========================================
- Hits 9654 9556 -98
+ Misses 1275 1274 -1
Partials 81 81 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
801802f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://944f8db0.egg-v3.pages.dev |
| Branch Preview URL: | https://dont-run-ets.egg-v3.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant refactoring that removes the egg-ts-helper dependency and all related code, configuration, and documentation. The changes are well-executed and cover all aspects of the removal, from dependency management in pnpm-lock.yaml and package.json files to updating documentation and cleaning up related scripts and tests. The removal of this deprecated tool is a good improvement for the project's maintainability. I have one minor suggestion for improving the clarity of the English documentation after the removal of content related to egg-ts-helper.
| "@eggjs/mock": "workspace:*", | ||
| "@eggjs/supertest": "workspace:*", | ||
| "@eggjs/tsconfig": "workspace:*", | ||
| "@types/node": "catalog:", | ||
| "@types/mocha": "catalog:", | ||
| "@swc-node/register": "catalog:", | ||
| "@swc/core": "catalog:", | ||
| "@types/mocha": "catalog:", | ||
| "@types/node": "catalog:", | ||
| "assert-file": "catalog:", | ||
| "coffee": "catalog:", | ||
| "cpy": "catalog:", |
There was a problem hiding this comment.
While not an error, sorting devDependencies alphabetically would improve readability and maintainability, making it easier to find specific packages.
"@eggjs/mock": "workspace:*",
"@eggjs/supertest": "workspace:*",
"@eggjs/tsconfig": "workspace:*",
"@swc-node/register": "catalog:",
"@swc/core": "catalog:",
"@types/mocha": "catalog:",
"@types/node": "catalog:",
"assert-file": "catalog:",
"coffee": "catalog:",
"cpy": "catalog:"![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
tools/egg-bin/package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{package.json,packages/**/package.json,plugins/**/package.json,tools/**/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
All packages must require Node.js >= 22.18.0 (set engines.node to ">=22.18.0")
Files:
tools/egg-bin/package.json
{packages/**/package.json,plugins/**/package.json,tools/**/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
{packages/**/package.json,plugins/**/package.json,tools/**/package.json}: Use workspace:* for internal dependencies between monorepo packages
Use catalog: versions for external dependencies defined in pnpm-workspace.yaml
Include script "typecheck": "tsc --noEmit" in all packages
Include script "lint": "oxlint --type-aware" in all packages
Include script "lint:fix" that runs oxlint with --fix (e.g., "pnpm run lint -- --fix")
Files:
tools/egg-bin/package.json
🧠 Learnings (6)
📚 Learning: 2025-10-23T17:37:37.549Z
Learnt from: CR
PR: eggjs/egg#0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.549Z
Learning: Applies to tegg/{core,plugin,standalone}/*/package.json : Use workspace:* for internal monorepo dependencies and catalog: for external dependencies
Applied to files:
tools/egg-bin/package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to plugins/*/package.json : Plugins must declare peerDependencies: { "egg": "workspace:*" }
Applied to files:
tools/egg-bin/package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/cookies/test/**/*.test.ts,tools/egg-bin/test/**/*.test.ts} : Mocha test suites (cookies, egg-bin) must use test/**/*.test.ts
Applied to files:
tools/egg-bin/package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to plugins/*/package.json : Plugins must include scripts: build (tsdown && rimraf dist *.tsbuildinfo && tsc -p tsconfig.build.json), typecheck (tsc --noEmit), lint (oxlint --type-aware), test (vitest run), prepublishOnly (pnpm run build)
Applied to files:
tools/egg-bin/package.json
📚 Learning: 2025-10-23T17:37:37.549Z
Learnt from: CR
PR: eggjs/egg#0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.549Z
Learning: Applies to tegg/core/*/package.json : Core packages should include scripts: typecheck, clean, build (tsc --noEmit, rimraf, build pipeline)
Applied to files:
tools/egg-bin/package.json
📚 Learning: 2025-10-18T11:59:58.226Z
Learnt from: CR
PR: eggjs/egg#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T11:59:58.226Z
Learning: Applies to {packages/**/package.json,plugins/**/package.json,tools/**/package.json} : Include script "typecheck": "tsc --noEmit" in all packages
Applied to files:
tools/egg-bin/package.json
⏰ 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). (4)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test bin (windows-latest, 22, 1/3)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
tools/egg-bin/package.json (2)
25-64: Verify appropriateness of relocated and newly-added dependencies after egg-ts-helper removal.The PR successfully removes egg-ts-helper integration and replaces it with direct tsdown/typescript dependencies in devDependencies (lines 63-64). The addition of runscript (line 62), @types/mocha, and @types/node (lines 52-53) should be verified to ensure they align with the build and test workflow changes introduced by this refactor.
Please confirm:
- That runscript, tsdown, and typescript are being actively used in the build/dev workflow
- That @types/mocha and @types/node are needed given the removal of egg-ts-helper
74-74: Verify clean script updates align with build artifacts.The clean script now includes
*.tsbuildinfo(line 74), which is appropriate for TypeScript build cleanup. Confirm that all tsbuildinfo artifacts generated during the build process will be properly removed.
1 participant
Summary by CodeRabbit