refactor: use definePluginFactory to export plugin config #5660
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the plugin configuration mechanism by introducing definePluginFactory to standardize how plugins export their configuration metadata. Previously, plugins declared metadata in package.json under eggPlugin field; now they export a factory function created via definePluginFactory with metadata inline in code.
Key changes:
- Plugins now export default factory functions containing plugin metadata (name, enable, path, dependencies)
- Removed
eggPluginfields from all plugin package.json files - Updated plugin registration in
packages/egg/src/config/plugin.tsto use imported factory functions with spread syntax - Updated documentation to reflect new plugin usage patterns
Reviewed Changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plugins/*/src/index.ts | Added default export using definePluginFactory with plugin metadata |
| plugins/*/package.json | Removed eggPlugin configuration object |
| plugins/*/README.md | Updated usage examples to show new plugin import and spread syntax |
| packages/egg/src/config/plugin.ts | Changed from object literals to imported plugin factories with spread syntax |
| packages/egg/src/lib/types.plugin.ts | Removed explicit plugin type imports (now handled by plugin factories) |
| site/docs/**/*.md | Updated documentation links to point to monorepo plugin locations |
| plugins/watcher/test/snapshots/*.snap | Updated test snapshots to include new default export |
| i18n: { | ||
| // 默认语言，默认 "en_US" | ||
| defaultLocale: 'zh-CN', | ||
| defaultLocale: 'zh_CN', |
There was a problem hiding this comment.
Changing the example default locale from 'zh-CN' to 'zh_CN' may be incorrect. The standard locale format typically uses hyphens (e.g., zh-CN), not underscores. Verify this matches the actual i18n implementation's expected format.
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 44 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 (1)
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 rewires many built-in plugins from static { enable/package } declarations to definePluginFactory-based factories (spread into config), removes per-plugin Changes
Sequence Diagram(s)sequenceDiagram
participant Config as packages/egg/config/plugin.ts
participant PluginFactory as plugin factory module
participant PluginPkg as plugin package (src/index.ts)
rect rgb(240,240,255)
note right of Config: Old flow — static declaration
Config->>Config: register { enable: true, package: '@eggjs/onerror' }
end
rect rgb(235,255,235)
note right of Config: New flow — factory-based
Config->>PluginFactory: import onerrorPlugin
Config->>PluginFactory: evaluate ...onerrorPlugin()
PluginFactory->>PluginPkg: definePluginFactory({ name, enable, path, deps })
PluginPkg-->>PluginFactory: EggPluginFactory object
PluginFactory-->>Config: spread plugin metadata into config
Config->>Config: register plugin from factory
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to focus on:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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 introduces a significant refactoring to the plugin system within Egg.js, moving towards a more modern and self-contained approach for defining and configuring plugins. By leveraging a new 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
|
Deploying egg-v3 with
|
| Latest commit: |
724227d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2e4e7400.egg-v3.pages.dev |
| Branch Preview URL: | https://refactor-plugin-exports.egg-v3.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5660 +/- ##
=======================================
Coverage 87.59% 87.59%
=======================================
Files 561 561
Lines 10911 10911
Branches 1231 1231
=======================================
Hits 9557 9557
Misses 1273 1273
Partials 81 81 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying egg with
|
| Latest commit: |
724227d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c75c7946.egg-cci.pages.dev |
| Branch Preview URL: | https://refactor-plugin-exports.egg-cci.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 introduces a significant and valuable refactoring by using a new definePluginFactory to export plugin configurations. This greatly improves consistency and simplifies how plugins are defined and used within the Egg.js ecosystem. The changes are applied consistently across numerous plugins, and the related documentation has been updated accordingly. I've found a few minor issues in the documentation that could be improved for clarity and correctness. Overall, this is a great step forward for the framework's maintainability.
| ```ts | ||
| // {app_root}/config/plugin.ts | ||
|
|
||
| export default { | ||
| jsonp: { | ||
| enable: true, | ||
| package: '@eggjs/jsonp', | ||
| }, | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
The documentation states that the jsonp plugin is enabled by default, but it's followed by a code example on how to enable it. This is contradictory and might confuse users. To be consistent with other built-in plugins' documentation (like i18n), it would be clearer to remove this example. The configuration options are detailed in the next section, which is sufficient.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/watcher/src/index.ts (1)
5-9: Consider adding JSDoc for consistency.Other plugins in this PR include usage documentation comments (e.g., i18n, static, logrotator, onerror). Consider adding similar JSDoc to maintain consistency across the plugin suite.
Example format:
+/** + * Watcher plugin + * + * @since 4.1.0 + * Usage: + * ```ts + * // config/plugin.ts + * import watcherPlugin from '@eggjs/watcher'; + * + * export default { + * ...watcherPlugin(), + * }; + * ``` + */ export default definePluginFactory({packages/egg/src/config/plugin.ts (1)
109-147: Consider migrating tegg plugins to factory pattern for consistency.The tegg plugins still use static object declarations while the refactor migrated other plugins to the factory pattern. This creates an inconsistency in the plugin registration approach.
Consider whether tegg plugins should also be migrated to use
definePluginFactoryfor consistency, or document why they require different treatment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
plugins/view-nunjucks/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snapplugins/watcher/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (46)
packages/egg/src/config/plugin.ts(3 hunks)packages/egg/src/lib/types.plugin.ts(0 hunks)plugins/development/README.md(0 hunks)plugins/i18n/README.md(3 hunks)plugins/i18n/package.json(0 hunks)plugins/i18n/src/index.ts(1 hunks)plugins/jsonp/README.md(3 hunks)plugins/jsonp/package.json(0 hunks)plugins/jsonp/src/index.ts(1 hunks)plugins/logrotator/README.md(3 hunks)plugins/logrotator/README.zh-CN.md(0 hunks)plugins/logrotator/package.json(0 hunks)plugins/logrotator/src/index.ts(1 hunks)plugins/multipart/package.json(1 hunks)plugins/multipart/src/index.ts(1 hunks)plugins/onerror/README.md(2 hunks)plugins/onerror/package.json(0 hunks)plugins/onerror/src/index.ts(1 hunks)plugins/redis/README.md(1 hunks)plugins/redis/package.json(0 hunks)plugins/redis/src/index.ts(1 hunks)plugins/schedule/package.json(0 hunks)plugins/schedule/src/index.ts(2 hunks)plugins/schedule/src/types.ts(1 hunks)plugins/security/README.md(0 hunks)plugins/security/package.json(0 hunks)plugins/security/src/index.ts(1 hunks)plugins/session/README.md(1 hunks)plugins/session/package.json(0 hunks)plugins/session/src/index.ts(1 hunks)plugins/static/package.json(0 hunks)plugins/static/src/index.ts(1 hunks)plugins/typebox-validate/README.md(1 hunks)plugins/typebox-validate/package.json(0 hunks)plugins/typebox-validate/src/index.ts(1 hunks)plugins/view-nunjucks/README.md(1 hunks)plugins/view-nunjucks/package.json(0 hunks)plugins/view-nunjucks/src/index.ts(1 hunks)plugins/view/package.json(0 hunks)plugins/view/src/index.ts(1 hunks)plugins/watcher/package.json(0 hunks)plugins/watcher/src/index.ts(1 hunks)site/docs/core/error-handling.md(1 hunks)site/docs/core/logger.md(1 hunks)site/docs/zh-CN/core/error-handling.md(1 hunks)site/docs/zh-CN/core/logger.md(1 hunks)
💤 Files with no reviewable changes (17)
- plugins/session/package.json
- plugins/jsonp/package.json
- plugins/security/README.md
- plugins/logrotator/README.zh-CN.md
- plugins/view-nunjucks/package.json
- plugins/i18n/package.json
- plugins/schedule/package.json
- plugins/logrotator/package.json
- plugins/development/README.md
- plugins/watcher/package.json
- plugins/redis/package.json
- plugins/security/package.json
- plugins/onerror/package.json
- plugins/view/package.json
- plugins/typebox-validate/package.json
- plugins/static/package.json
- packages/egg/src/lib/types.plugin.ts
🧰 Additional context used
📓 Path-based instructions (9)
{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:
plugins/multipart/package.json
plugins/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
plugins/*/package.json: Plugin package.json must follow the standard exports/publishConfig mapping (dev exports to ./src/.ts, publishConfig.exports to ./dist/.js)
Plugins must declare peerDependencies: { "egg": "workspace:*" }
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)
Files:
plugins/multipart/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:
plugins/multipart/package.json
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
plugins/schedule/src/types.tsplugins/static/src/index.tsplugins/typebox-validate/src/index.tsplugins/watcher/src/index.tsplugins/security/src/index.tsplugins/schedule/src/index.tsplugins/onerror/src/index.tsplugins/session/src/index.tsplugins/logrotator/src/index.tsplugins/multipart/src/index.tsplugins/redis/src/index.tsplugins/jsonp/src/index.tsplugins/view/src/index.tsplugins/i18n/src/index.tsplugins/view-nunjucks/src/index.tspackages/egg/src/config/plugin.ts
{packages/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{packages/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts: For isolatedDeclarations support, all exported functions/methods/getters must have explicit return type annotations
Avoid computed property names with symbols in class declarations (no get SYM)
Add explicit type annotations for class properties when needed (no inferred exported property types)
Exported symbols must use unique symbol type (e.g., export const X: unique symbol = Symbol('x'))
Files:
plugins/schedule/src/types.tsplugins/static/src/index.tsplugins/typebox-validate/src/index.tsplugins/watcher/src/index.tsplugins/security/src/index.tsplugins/schedule/src/index.tsplugins/onerror/src/index.tsplugins/session/src/index.tsplugins/logrotator/src/index.tsplugins/multipart/src/index.tsplugins/redis/src/index.tsplugins/jsonp/src/index.tsplugins/view/src/index.tsplugins/i18n/src/index.tsplugins/view-nunjucks/src/index.tspackages/egg/src/config/plugin.ts
plugins/*/src/types.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All plugins must provide src/types.ts that augments module 'egg' (module augmentation, config types, interface extensions, .ts import extensions, documented properties)
Files:
plugins/schedule/src/types.ts
site/**
📄 CodeRabbit inference engine (AGENTS.md)
When working on the documentation site, scrub generated content before committing to avoid leaking local URLs
Files:
site/docs/zh-CN/core/logger.mdsite/docs/core/logger.mdsite/docs/zh-CN/core/error-handling.mdsite/docs/core/error-handling.md
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use TypeScript for all source files in packages (no JavaScript source files)
Files:
packages/egg/src/config/plugin.ts
packages/egg/src/config/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store default configurations and plugin configs under packages/egg/src/config/
Files:
packages/egg/src/config/plugin.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: eggjs/egg#0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-10-23T17:37:37.549Z
Learning: Applies to tegg/plugin/*/package.json : Plugin packages must define eggPlugin in package.json with dependencies
📚 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/plugin/*/package.json : Plugin packages must define eggPlugin in package.json with dependencies
Applied to files:
plugins/multipart/package.jsonplugins/jsonp/README.mdplugins/jsonp/src/index.tspackages/egg/src/config/plugin.ts
📚 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/*/src/types.ts : All plugins must provide src/types.ts that augments module 'egg' (module augmentation, config types, interface extensions, .ts import extensions, documented properties)
Applied to files:
plugins/static/src/index.tsplugins/typebox-validate/src/index.tsplugins/watcher/src/index.tsplugins/security/src/index.tsplugins/schedule/src/index.tsplugins/onerror/src/index.tsplugins/session/src/index.tsplugins/multipart/src/index.tsplugins/redis/src/index.tsplugins/jsonp/src/index.tsplugins/view/src/index.tsplugins/i18n/src/index.tsplugins/view-nunjucks/src/index.tspackages/egg/src/config/plugin.ts
📚 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/plugin/*/{test,tests}/**/*.ts : Add Vitest tests for plugins using eggjs/mock; tests will be discovered by the root vitest config
Applied to files:
plugins/schedule/src/index.ts
🧬 Code graph analysis (14)
plugins/static/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/typebox-validate/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/watcher/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/security/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/schedule/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/onerror/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/session/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/logrotator/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/multipart/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/redis/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/jsonp/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/view/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/i18n/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
plugins/view-nunjucks/src/index.ts (1)
packages/egg/src/lib/define.ts (2)
definePluginFactory(91-103)EggPluginFactory(73-73)
⏰ 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). (20)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Test (windows-latest, 22, 1/5)
- GitHub Check: Test (windows-latest, 24, 2/5)
- GitHub Check: Test (windows-latest, 22, 3/5)
- GitHub Check: Test (ubuntu-latest, 24, 4/5)
- GitHub Check: Test (ubuntu-latest, 22, 2/5)
- GitHub Check: Test (macos-latest, 22, 2/5)
- GitHub Check: Test (macos-latest, 22, 1/5)
- GitHub Check: Test (ubuntu-latest, 24, 1/5)
- GitHub Check: Test (ubuntu-latest, 22, 5/5)
- GitHub Check: Test (windows-latest, 22, 4/5)
- GitHub Check: Test (ubuntu-latest, 22, 4/5)
- GitHub Check: Test (windows-latest, 22, 2/5)
- GitHub Check: Test (macos-latest, 24, 5/5)
- GitHub Check: Test bin (ubuntu-latest, 22, 1/3)
- GitHub Check: Test bin (windows-latest, 22, 2/3)
- GitHub Check: Test bin (windows-latest, 22, 1/3)
- GitHub Check: Test bin (ubuntu-latest, 22, 0/3)
- GitHub Check: Test bin (windows-latest, 22, 0/3)
🔇 Additional comments (37)
plugins/schedule/src/types.ts (2)
1-14: ✓ Module augmentation is properly structured and documentation URL is updated.The file correctly augments the 'egg' module with the schedule plugin's types and configuration. The documentation URL update on Line 11 from a locale-specific path to the default English path aligns with the PR's plugin factory refactor effort and improves discoverability. All interface extensions are properly documented and type-annotated.
15-42: ✓ Agent and Application interface augmentations are properly typed.The Agent interface correctly augments with ScheduleStrategy, TimerScheduleStrategy, and schedule properties, all properly documented and typed. The Application interface augmentation for scheduleWorker and runSchedule follows consistent patterns with explicit type annotations and JSDoc comments.
plugins/typebox-validate/README.md (2)
117-122: Plugin configuration example correctly demonstrates the factory pattern.The updated configuration now properly reflects the shift from static object declarations to the dynamic factory pattern. The import-and-spread approach (
...typeboxValidatePlugin()) aligns well with the PR'sdefinePluginFactory()refactor.
109-112: No issues found—README documentation is accurate and complete.The egg-typebox-validate repository link (line 112) is active and accessible. The factory pattern documented in lines 117-122 matches the actual plugin export implementation, and the installation command is correctly modernized. All referenced URLs and configuration examples are verified and correct.
plugins/session/README.md (1)
21-21: LGTM!The documentation correctly refers to the plugin by its plugin name "session" rather than the package name, aligning with the built-in plugin factory pattern.
plugins/static/src/index.ts (1)
5-23: LGTM!The plugin factory implementation is correct and well-documented. The JSDoc provides clear usage instructions for consumers.
plugins/jsonp/README.md (2)
20-20: LGTM!The documentation correctly emphasizes that jsonp is a built-in plugin and enabled by default, aligning with the plugin factory refactoring.
103-120: Documentation structure improved.The addition of "Standard Application Usage" and "tegg HttpController Usage" sections improves clarity. The TODO placeholder for tegg usage is reasonable for future documentation.
plugins/i18n/src/index.ts (1)
5-23: LGTM!The i18n plugin factory implementation is correct and well-documented, following the established pattern in this PR.
plugins/multipart/package.json (1)
4-4: LGTM!The description capitalization is a minor improvement. Note that this package is part of the broader migration away from
eggPluginmetadata in package.json toward thedefinePluginFactorypattern, which is the intended architectural change in this PR.plugins/logrotator/src/index.ts (1)
5-24: LGTM!The logrotator plugin factory is correctly implemented with proper documentation. The dependency on 'schedule' is appropriately specified.
plugins/onerror/src/index.ts (1)
5-24: LGTM!The onerror plugin factory is correctly implemented with proper documentation. The use of
optionalDependenciesfor 'jsonp' appropriately indicates that jsonp integration is optional.plugins/onerror/README.md (2)
21-21: LGTM! Documentation phrasing improved.The updated phrasing "onerror plugin is enabled by default" is clearer and more consistent with the plugin factory pattern being adopted across the codebase.
31-40: LGTM! TypeScript configuration example is accurate.The updated example correctly demonstrates the modern TS-based configuration using
defineConfig, which aligns with the project's shift to TypeScript and the plugin factory pattern.plugins/security/src/index.ts (1)
1-24: LGTM! Plugin factory pattern implemented correctly.The security plugin correctly uses
definePluginFactorywith all required metadata (name, enable, path, optionalDependencies). The type assertionas EggPluginFactoryensures explicit typing for isolatedDeclarations support as per coding guidelines.plugins/jsonp/src/index.ts (1)
1-24: LGTM! Consistent plugin factory implementation.The JSONP plugin correctly follows the same factory pattern as other plugins, with proper metadata and explicit typing for isolatedDeclarations support.
plugins/view-nunjucks/src/index.ts (1)
1-17: LGTM! Plugin factory with appropriate named exports.The Nunjucks plugin correctly implements the factory pattern while also exposing necessary implementation classes and types. Using
dependencies(rather thanoptionalDependencies) for security and view is appropriate since these are required for Nunjucks functionality.plugins/view-nunjucks/README.md (1)
26-32: LGTM! Documentation correctly reflects plugin factory pattern.The updated usage example accurately demonstrates how to use the new plugin factory pattern with
nunjucksPlugin(), consistent with the broader refactoring across all plugins.site/docs/zh-CN/core/error-handling.md (1)
61-61: LGTM! Link updated to point to specific plugin location.The updated link now points directly to the onerror plugin directory, improving documentation accuracy and user navigation.
site/docs/core/error-handling.md (1)
64-64: LGTM! Link updated consistently with Chinese docs.The English documentation now correctly references the specific onerror plugin location, maintaining consistency with the Chinese translation.
site/docs/zh-CN/core/logger.md (1)
375-375: LGTM! Link updated to monorepo plugin location.The updated link correctly points to the logrotator plugin within the monorepo structure, consistent with other documentation updates in this PR.
site/docs/core/logger.md (1)
372-372: LGTM! Link update aligns with monorepo structure.The logrotator reference now correctly points to the plugin location within the main egg repository on the next branch.
plugins/typebox-validate/src/index.ts (1)
4-24: LGTM! Clean plugin factory implementation.The implementation correctly follows the plugin factory pattern with:
- Proper imports and type annotations
- Clear JSDoc documentation with usage example
- Correct metadata (name, enable, path)
- Type assertion for isolatedDeclarations support
plugins/redis/src/index.ts (1)
3-23: LGTM! Consistent plugin factory implementation.The Redis plugin correctly adopts the factory pattern, maintaining consistency with other plugins in the refactor.
plugins/redis/README.md (1)
39-43: LGTM! Documentation accurately reflects the new plugin factory pattern.The example correctly demonstrates importing and using the plugin factory with the spread operator.
plugins/schedule/src/index.ts (1)
3-19: LGTM! Plugin factory implementation preserves existing exports.The schedule plugin correctly adopts the factory pattern while maintaining all existing named exports, ensuring backward compatibility.
plugins/view/src/index.ts (1)
3-11: LGTM! Clean and minimal plugin factory implementation.The view plugin correctly implements the factory pattern with proper type annotations and maintains existing re-exports.
plugins/i18n/README.md (2)
35-35: Verify locale format consistency across documentation.The default locale format changed from
'zh-CN'(hyphen) to'zh_CN'(underscore). However, lines 137-139 still show hyphenated examples:en-US,zh-TW,zh-CN. Ensure this format change is intentional and consistent throughout the documentation and codebase.
109-121: LGTM! Helpful addition of HttpController usage example.The new section clearly demonstrates how to use i18n functionality with the decorator-based controller syntax, complementing the existing usage examples.
plugins/multipart/src/index.ts (1)
3-24: LGTM! Plugin factory correctly specifies optional dependencies.The multipart plugin implementation properly declares its optional dependency on the schedule plugin, demonstrating the factory pattern's support for plugin relationships.
plugins/logrotator/README.md (3)
16-16: LGTM: Accurate built-in plugin status.The documentation correctly reflects that logrotator is now enabled by default as a built-in plugin, removing unnecessary installation/enablement instructions.
20-36: LGTM: Proper TypeScript configuration example.The configuration example correctly demonstrates the TypeScript-first approach with appropriate export syntax and nested structure.
64-94: LGTM: Proper TypeScript type annotations.The customization example correctly adds TypeScript type annotations (
app: Application) while preserving the original functionality.packages/egg/src/config/plugin.ts (3)
16-16: LGTM: Correct environment variable handling.The
enableTeggPluginsflag correctly handles the environment variable with proper string comparison. The logic enables tegg plugins by default unless explicitly disabled.
1-12: No issues found – plugin factories properly export with explicit return types.Verification confirms all 12 plugins use
definePluginFactory()which has an explicit return type annotation (EggPluginFactory) atpackages/egg/src/lib/define.ts:91. The return type(options?: EggPluginOptions) => Record<string, EggPluginMeta>is compatible with the plugin config spread pattern and satisfies theisolatedDeclarationsrequirement for explicit return type annotations.
18-106: No issues found with plugin factory refactor.Verification of
definePluginFactoryimplementation confirms all original concerns are addressed:
- Factory return structure ✅: Each factory returns
{ [pluginName]: EggPluginItem }(line 94-95 inpackages/egg/src/lib/define.ts)- No key collisions ✅: Each factory generates exactly one property keyed by its unique
name, eliminating collision risk- Single entry per factory ✅: The returned object contains exactly one property — the plugin name
The spread-based refactor is type-safe and safe at runtime.
plugins/session/src/index.ts (1)
7-11: Review comment is incorrect—ignore this suggestion.The type assertion
as EggPluginFactoryis not redundant. It's a deliberate, codebase-wide pattern applied consistently across all 16+ plugin files and serves the coding guideline requirement for "explicit return type annotations" on exported symbols in theplugins/**directory. Additionally, the project's minimum Node.js version (22.18.0) fully supportsimport.meta.dirname(requires 20.11.0+), so there are no compatibility concerns.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/development/src/index.ts (1)
24-30: Remove the unnecessary type assertion on line 30.
definePluginFactoryalready returnsEggPluginFactory, making theas EggPluginFactoryassertion redundant and safe to remove:-}) as EggPluginFactory; +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/egg/src/lib/types.plugin.ts(1 hunks)packages/egg/test/lib/core/loader/load_plugin.test.ts(1 hunks)plugins/development/src/index.ts(1 hunks)plugins/redis/README.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/redis/README.md
🧰 Additional context used
📓 Path-based instructions (7)
packages/**/test/**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/test/**/*.test.ts: Name test files as test/**/*.test.ts and run them with Vitest
Use import { describe, it } from 'vitest' in tests
Use Node.js built-in assert module for test assertions
Files:
packages/egg/test/lib/core/loader/load_plugin.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Prefer TypeScript and ESM: write sources and exports in .ts (ESM-first) rather than CommonJS
Use two-space indentation, trailing commas, and semicolons (Prettier/oxlint defaults)
Name files in lowercase with hyphens (e.g., loader-context.ts)
Name classes in PascalCase
Name functions and variables in camelCase
Re-export types thoughtfully to keep the public API stable
Files:
packages/egg/test/lib/core/loader/load_plugin.test.tsplugins/development/src/index.tspackages/egg/src/lib/types.plugin.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/test/**/*.test.ts: Place test suites following Vitest discovery: /test//*.test.ts
Mirror the repository test pattern when adding new suites
Files:
packages/egg/test/lib/core/loader/load_plugin.test.ts
{packages/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
{packages/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts: For isolatedDeclarations support, all exported functions/methods/getters must have explicit return type annotations
Avoid computed property names with symbols in class declarations (no get SYM)
Add explicit type annotations for class properties when needed (no inferred exported property types)
Exported symbols must use unique symbol type (e.g., export const X: unique symbol = Symbol('x'))
Files:
packages/egg/test/lib/core/loader/load_plugin.test.tsplugins/development/src/index.tspackages/egg/src/lib/types.plugin.ts
{packages/!(cookies)/**/test/**/*.test.ts,plugins/**/test/**/*.test.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Vitest test files must follow the naming pattern test/**/*.test.ts
Files:
packages/egg/test/lib/core/loader/load_plugin.test.ts
{packages/**/test/**,plugins/**/test/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Node.js assert for assertions in Vitest tests
Files:
packages/egg/test/lib/core/loader/load_plugin.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use TypeScript for all source files in packages (no JavaScript source files)
Files:
packages/egg/src/lib/types.plugin.ts
🧠 Learnings (3)
📚 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/*/src/types.ts : All plugins must provide src/types.ts that augments module 'egg' (module augmentation, config types, interface extensions, .ts import extensions, documented properties)
Applied to files:
plugins/development/src/index.tspackages/egg/src/lib/types.plugin.ts
📚 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/plugin/*/package.json : Plugin packages must define eggPlugin in package.json with dependencies
Applied to files:
plugins/development/src/index.ts
📚 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/**,plugins/**,tools/!(egg-bin)/**}/**/*.ts : Add explicit type annotations for class properties when needed (no inferred exported property types)
Applied to files:
packages/egg/src/lib/types.plugin.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). (21)
- GitHub Check: Test (ubuntu-latest, 22, 5/5)
- GitHub Check: Test (windows-latest, 22, 1/5)
- GitHub Check: Test (ubuntu-latest, 24, 3/5)
- GitHub Check: Test (windows-latest, 24, 4/5)
- GitHub Check: Test (windows-latest, 22, 3/5)
- GitHub Check: Test (ubuntu-latest, 22, 2/5)
- GitHub Check: Test (macos-latest, 24, 3/5)
- GitHub Check: Test (windows-latest, 24, 1/5)
- GitHub Check: Test (macos-latest, 24, 2/5)
- GitHub Check: Test (ubuntu-latest, 22, 3/5)
- GitHub Check: Test (macos-latest, 24, 4/5)
- GitHub Check: Test (macos-latest, 22, 3/5)
- GitHub Check: Test bin (windows-latest, 22, 0/3)
- GitHub Check: Test bin (ubuntu-latest, 22, 2/3)
- GitHub Check: Test bin (windows-latest, 22, 2/3)
- GitHub Check: Test bin (windows-latest, 22, 1/3)
- GitHub Check: Test bin (ubuntu-latest, 22, 1/3)
- GitHub Check: Test bin (ubuntu-latest, 22, 0/3)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (2)
plugins/development/src/index.ts (1)
3-4: LGTM! Import order and syntax are correct.The import statement correctly places module augmentation (
./types.ts) first and uses thetypekeyword for type-only imports, which aids tree-shaking.packages/egg/src/lib/types.plugin.ts (1)
1-3: Approved — no changes needed.The import of
@eggjs/development/typesis correct; this is an external scoped package that ships with built-in TypeScript declarations. The comment update clearly explains the purpose of enabling built-in plugin type augmentation.Likely an incorrect or invalid review comment.
1 participant
Summary by CodeRabbit
Refactor
Documentation
Tests
Chores