feat(tegg): declarative module plugin mechanism#6021
Open
gxkl wants to merge 71 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds declarative inner-object and lifecycle decorators, routes prototype resolution through new metadata and graph utilities, introduces inner-object runtime/load-unit support, migrates AOP/config/DAL and tegg boot wiring to the new model, replaces the standalone Runner with StandaloneApp, and updates supporting tests and docs. ChangesTegg Module Plugin Mechanism
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Deploying egg with
|
| Latest commit: |
d49c104
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9faaeeff.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-module-plugin-core.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## next #6021 +/- ##
===========================================
- Coverage 81.95% 70.88% -11.08%
===========================================
Files 678 18 -660
Lines 20800 735 -20065
Branches 4147 181 -3966
===========================================
- Hits 17047 521 -16526
+ Misses 3235 181 -3054
+ Partials 518 33 -485 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
Deploying egg-v3 with
|
| Latest commit: |
d49c104
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c813e992.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-module-plugin-core.egg-v3.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements a declarative module plugin mechanism using @InnerObjectProto and @EggLifecycleProto decorators, enabling framework extensions to be loaded into an InnerObjectLoadUnit before the business graph is built for both standalone and egg hosts. It also converts AOP, DAL, and ConfigSource hooks into module plugins and renames Runner to StandaloneApp. The review feedback suggests maintaining consistency by using .ts extensions instead of .js in import/export paths within aop-runtime, and defensively checking if caught exceptions are Error instances before accessing their message property in StandaloneApp.ts and main.ts to avoid runtime type errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+5
to
+6
| import { crossCutGraphHook } from './CrossCutGraphHook.js'; | ||
| import { pointCutGraphHook } from './PointCutGraphHook.js'; |
Contributor
There was a problem hiding this comment.
According to the repository's general rules, TypeScript source file imports should use the .ts extension instead of .js to maintain consistency across the codebase.
Suggested change
| import { crossCutGraphHook } from './CrossCutGraphHook.js'; | |
| import { pointCutGraphHook } from './PointCutGraphHook.js'; | |
| import { crossCutGraphHook } from './CrossCutGraphHook.ts'; | |
| import { pointCutGraphHook } from './PointCutGraphHook.ts'; |
References
- In this repository, use '.ts' extensions in import/export paths for TypeScript source files to maintain consistency with the existing convention across source and test files.
Contributor
Author
There was a problem hiding this comment.
不采纳这里的 .ts import 建议。tegg 源码当前按 ESM 输出规范使用 .js import specifier，同包其它 TS 源码也是这个模式；改成 .ts 反而会和现有构建/类型检查约定不一致。
Comment on lines
+4
to
+7
| import { AopGraphHookRegistrar } from './AopGraphHookRegistrar.js'; | ||
| import { EggObjectAopHook } from './EggObjectAopHook.js'; | ||
| import { EggPrototypeCrossCutHook } from './EggPrototypeCrossCutHook.js'; | ||
| import { LoadUnitAopHook } from './LoadUnitAopHook.js'; |
Contributor
There was a problem hiding this comment.
According to the repository's general rules, TypeScript source file imports should use the .ts extension instead of .js to maintain consistency across the codebase.
Suggested change
| import { AopGraphHookRegistrar } from './AopGraphHookRegistrar.js'; | |
| import { EggObjectAopHook } from './EggObjectAopHook.js'; | |
| import { EggPrototypeCrossCutHook } from './EggPrototypeCrossCutHook.js'; | |
| import { LoadUnitAopHook } from './LoadUnitAopHook.js'; | |
| import { AopGraphHookRegistrar } from './AopGraphHookRegistrar.ts'; | |
| import { EggObjectAopHook } from './EggObjectAopHook.ts'; | |
| import { EggPrototypeCrossCutHook } from './EggPrototypeCrossCutHook.ts'; | |
| import { LoadUnitAopHook } from './LoadUnitAopHook.ts'; |
References
- In this repository, use '.ts' extensions in import/export paths for TypeScript source files to maintain consistency with the existing convention across source and test files.
Contributor
Author
There was a problem hiding this comment.
不改，原因同上：tegg TS 源码使用 .js import specifier 才符合当前 ESM 输出约定。这个 thread 现在也已经是 outdated。
Comment on lines
+7
to
+8
| export * from './AopGraphHookRegistrar.js'; | ||
| export * from './AopInnerObjectClazzList.js'; |
Contributor
There was a problem hiding this comment.
According to the repository's general rules, TypeScript source file exports should use the .ts extension instead of .js to maintain consistency across the codebase.
Suggested change
| export * from './AopGraphHookRegistrar.js'; | |
| export * from './AopInnerObjectClazzList.js'; | |
| export * from './AopGraphHookRegistrar.ts'; | |
| export * from './AopInnerObjectClazzList.ts'; |
References
- In this repository, use '.ts' extensions in import/export paths for TypeScript source files to maintain consistency with the existing convention across source and test files.
Contributor
Author
There was a problem hiding this comment.
不改，原因同上：这里的 export specifier 也应保持 .js，和 tegg 现有 ESM 源码约定一致。这个 thread 现在已 outdated。
Comment on lines
+321
to
+324
| ctx.destroy(lifecycle).catch((e) => { | ||
| e.message = `[tegg/standalone] destroy tegg context failed: ${e.message}`; | ||
| console.warn(e); | ||
| }); |
Contributor
There was a problem hiding this comment.
When handling a caught exception, defensively check if the caught value is an Error instance before accessing or mutating its message property. Mutating e.message directly can throw a TypeError in strict mode if e is a primitive value. Use String(e) as a fallback to safely format the error message.
Suggested change
| ctx.destroy(lifecycle).catch((e) => { | |
| e.message = `[tegg/standalone] destroy tegg context failed: ${e.message}`; | |
| console.warn(e); | |
| }); | |
| ctx.destroy(lifecycle).catch((e) => { | |
| const msg = e instanceof Error ? e.message : String(e); | |
| console.warn('[tegg/standalone] destroy tegg context failed: ' + msg); | |
| }); |
References
- When creating a new Error from a caught exception, check if the caught value is an 'Error' instance before accessing its 'message' property. Use 'String(err)' as a fallback for non-Error values to ensure a meaningful error message.
Contributor
Author
There was a problem hiding this comment.
已按这个方向处理：destroy catch 里不再直接假设 caught value 一定是 Error；会先做 Error 判断，非 Error 用 String(e) 兜底格式化，避免 primitive throw 时二次抛错。这个 thread 目前已 outdated。
Comment on lines
32
to
35
| app.destroy().catch((e) => { | ||
| e.message = `[tegg/standalone] destroy tegg failed: ${e.message}`; | ||
| console.warn(e); | ||
| }); |
Contributor
There was a problem hiding this comment.
When handling a caught exception, defensively check if the caught value is an Error instance before accessing or mutating its message property. Mutating e.message directly can throw a TypeError in strict mode if e is a primitive value. Use String(e) as a fallback to safely format the error message.
Suggested change
| app.destroy().catch((e) => { | |
| e.message = `[tegg/standalone] destroy tegg failed: ${e.message}`; | |
| console.warn(e); | |
| }); | |
| app.destroy().catch((e) => { | |
| const msg = e instanceof Error ? e.message : String(e); | |
| console.warn('[tegg/standalone] destroy tegg failed: ' + msg); | |
| }); |
References
- When creating a new Error from a caught exception, check if the caught value is an 'Error' instance before accessing its 'message' property. Use 'String(err)' as a fallback for non-Error values to ensure a meaningful error message.
Contributor
Author
There was a problem hiding this comment.
已处理：main 成功/失败路径里都会等待 app destroy，并且 destroy catch 对非 Error 值做 String(e) 兜底，不再直接改 e.message。这个 thread 目前已 outdated。
gxkl
changed the title
Introduce the declarative framework-extension (module plugin) core so a plain tegg module can provide lifecycle hooks and framework inner objects via decorators, instead of the host hand-registering every hook in its boot code. Ported from eggjs/tegg#325 (standalone-next) and adapted to the next-branch architecture. - core-decorator/types: @InnerObjectProto (framework inner object, a SingletonProto with EGG_INNER_OBJECT_PROTO_IMPL_TYPE) and @EggLifecycleProto with the five typed variants (LoadUnit/LoadUnitInstance/EggPrototype/EggObject/EggContext); PrototypeUtil metadata accessors. - loader: divert inner object classes into ModuleDescriptor.innerObjectClazzList; getDecoratedFiles now covers the new list so bundle/manifest mode still re-imports those files. - metadata: EggInnerObjectPrototypeImpl (resolves injects at create time); InjectObjectPrototypeFinder extracted from EggPrototypeBuilder (behavior unchanged); ProtoGraphUtils extracted from GlobalGraph with a proto-name index replacing the O(n*m*n) scan; ProtoDescriptorHelper.createByInstanceClazz supports define/instance module separation. - runtime: EggInnerObjectImpl runs only decorator-declared self lifecycle methods (hook-callback names never double as self lifecycle); host-agnostic InnerObjectLoadUnit(+Instance/Builder) instantiates inner objects on a dedicated topologically-sorted proto graph (cycle detection, hard error on missing non-optional deps) and auto registers/deregisters lifecycle protos by declared type via the scope-aware lifecycle utils. The InnerObjectLoadUnit is designed to be instantiated before the business GlobalGraph build so registered hooks (including future declarative graph build hooks) land inside their consumption windows; host wiring for standalone/app follows in separate PRs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rename Runner to StandaloneApp (no Runner alias kept — 4.x breaking change; main()/preLoad() entries are unchanged) and restructure boot into explicit phases so module plugins work end to end: 1. scan modules + create the business GlobalGraph (nodes only) 2. create AND instantiate the InnerObjectLoadUnit — host-provided inner objects plus every scanned @InnerObjectProto/@XxxLifecycleProto class (topologically ordered); lifecycle protos auto-register 3. build()/sort() the business graph and create module load units 4. instantiate module load units Deferring build/sort until after the inner load unit is instantiated restores the tegg#325 two-phase ordering: hooks registered by lifecycle protos (including graph build hooks registered in @LifecyclePostInject) land before their consumption windows. Destroy now runs in reverse creation order so lifecycle protos stay registered until every object they may hook is torn down. Also: - new frameworkDeps option: framework module plugins scanned ahead of the app's own modules (service worker runtime packages plug in here) - bundle-mode support: EggModuleLoader accepts a tegg manifest + loaderFS (reuses precomputed decorated files instead of globbing) and StandaloneApp.loadMetadata() provides the scan-only manifest generation entry for bundlers - replace StandaloneLoadUnit / StandaloneInnerObjectProto / StandaloneInnerObject with the host-agnostic core implementations (InnerObjectLoadUnit / ProvidedInnerObjectProto) - port the tegg#325 module plugin test suite (five lifecycle proto types, inner object DI, PUBLIC/PRIVATE access semantics) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire the module plugin mechanism into the egg host, the part tegg#325 left out (inner object / lifecycle protos were silently ignored in app mode). ModuleHandler.init() now runs in phases: 1. EggModuleLoader.initGraph(): scan modules, create the business GlobalGraph (nodes only) and flush buffered build hooks 2. create AND instantiate the InnerObjectLoadUnit from every scanned module's innerObjectClazzList — lifecycle protos auto-register with DI wired, before any business load unit exists 3. EggModuleLoader.load(): APP load unit + build()/sort() + module load units — declarative graph build hooks registered in step 2 land before build() consumes them 4. instantiate business load units (inner unit skips egg compatible mounting) destroy() runs in reverse creation order so lifecycle protos stay registered until every object they may hook is torn down. The same module plugin package now behaves identically under the egg host and the standalone host. Covered by a fixture app whose module provides @InnerObjectProto/@LoadUnitLifecycleProto/@EggObjectLifecycleProto classes; MultiApp isolation regression stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Complete the module plugin bootstrap (the #handleCompatibility TODO from tegg#325): every hook the standalone host used to hand-register is now a declarative module plugin class, shared verbatim by both hosts. - aop: LoadUnitAopHook / EggPrototypeCrossCutHook / EggObjectAopHook are @XxxLifecycleProto classes injecting an @InnerObjectProto CrosscutAdviceFactory (optional constructor kept for manual paths); the crossCut/pointCut graph build hooks are registered by the new AopGraphHookRegistrar from @LifecyclePostInject — instantiated after graph creation and before build(), the only valid window. Exported as AOP_INNER_OBJECT_CLAZZ_LIST. - dal: the three hooks are @XxxLifecycleProto classes injecting the host-provided moduleConfigs / runtimeConfig / logger inner objects instead of constructor args. Exported as DAL_INNER_OBJECT_CLAZZ_LIST. - config source: both ConfigSourceLoadUnitHook copies decorated with @LoadUnitLifecycleProto. - LoadUnitMultiInstanceProtoHook registration dropped on both hosts — its preCreate is empty and the static set has no consumers. Host wiring: - ModuleHandler.registerInnerObjectClazzList() buffers plugin-provided classes (aop/dal boots now register one list in configDidLoad instead of constructing and registering hooks); the egg host provides moduleConfigs/runtimeConfig/logger as PRIVATE inner objects so they stay visible to inner objects only and never pollute cross-unit resolution (InnerObject/ProvidedInnerObjectProto gain accessLevel). - StandaloneApp feeds the built-in lists, always provides a logger inner object, and drops all manual register/delete bookkeeping — lifecycle protos deregister with the InnerObjectLoadUnit. - InnerObjectLoadUnitBuilder dedupes by class: hosts hard-feed built-in lists while the owning package may also be scanned as an eggModule (e.g. @eggjs/dal-plugin as a module dependency) — first add wins. - test-util LoaderUtil mirrors the production loader and diverts inner object classes out of module load units. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inner unit Review follow-ups on the module plugin PR: - ConfigSourceLoadUnitHook existed twice (standalone + egg plugin copy) — exactly the duplication the module plugin mechanism is meant to end. Move the single host-agnostic class to @eggjs/metadata (hook/); both hosts now feed the same import into their inner-object clazz lists. - ModuleHandler tracked the inner-object load unit inside the business loadUnits list, forcing INNER_OBJECT_LOAD_UNIT_TYPE filters in the init loop and contextModuleCompatible. Track it in its own field: business loops need no filtering, destroy still tears it down last (its lifecycle protos must outlive every hooked object). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up: hand-fed inner-object class lists defeated the module plugin's purpose — the packages already declare eggModule metadata, so consume them AS modules: - StandaloneApp: aop-runtime/dal-plugin join the regular module scan as built-in framework modules (their @InnerObjectProto/@XxxLifecycleProto classes are diverted into the InnerObjectLoadUnit by loadApp); the AOP_/DAL_INNER_OBJECT_CLAZZ_LIST exports are gone. Module references now dedupe by path (an app may also depend on a built-in module directly). - egg host: moduleHandler.registerInnerObjectModule(path) — the aop/dal plugins register their package as a scanned module instead of a class list; manifest collection includes registered framework modules. - aop-runtime re-exports CrosscutAdviceFactory (decorated in aop-decorator) so the scan picks it up as a module member, and no longer needs the manual list file. - LoaderUtil.filePattern: '!**/test' does not exclude descendants — add '!**/test/**' (and coverage) so scanning workspace packages that ship test dirs stays safe; built-in references also exclude test fixture modules from the reference scan. - Drop the test-only optional constructor params from LoadUnitAopHook / EggPrototypeCrossCutHook; the aop-runtime tests wire the dependency via Reflect.set on the DI property instead. registerInnerObjectClazzList stays for host-boot wiring (e.g. the shared ConfigSourceLoadUnitHook) and edge hosts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…the contract Second review round: - Drop registerInnerObjectModule (added one commit ago): a central register is exactly what module plugins should not need. The egg host resolves module plugins from what already exists: tegg-config's ModuleScanner picks eggModule-declaring packages up from app/framework dependencies, and EggModuleLoader.resolveModuleReferences() fills the remaining gap by auto-including every ENABLED plugin whose package declares eggModule (ModuleConfigUtil.hasEggModule). Enabling the plugin is the whole contract. - plugin/aop is now itself the AOP module (eggModule: teggAop) and re-exports the aop-runtime/aop-decorator hook classes for the scan; plugin/dal already declared eggModule - both app.ts registrations are gone. aop-runtime keeps its own eggModule for the standalone built-in. - InnerObjectLoadUnitBuilder: remove the #seenClazzSet dual-arrival dedupe - reference-level path dedupe covers the legitimate case now that hand-fed lists are gone; a genuine double registration fails loud. Host-provided instances now resolve through the SAME selectProto rules as graph protos (name + qualifiers + access level) instead of a bare name whitelist; they still skip the topological sort - an already-constructed instance has no construction order and no outgoing edges. - test-util buildGlobalGraph reuses LoaderFactory.loadApp for classification instead of re-implementing the inner-object diversion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in the graph Third review round: - Only plugins are modules: aop-runtime drops its eggModule declaration (pure library again); @eggjs/aop-plugin (eggModule: teggAop) is THE aop module for BOTH hosts - its egg imports are all type-only, so the standalone built-in reference now points at the plugin package, same as the egg host resolves it from the enabled-plugin list. One module name, one carrier. resolveModuleReferences locates the real package root from plugin.path (which points inside the package, through symlinks). - Host-provided inner objects are now REAL graph nodes: same descriptors, same vertices, same edge resolution and topological sort as hook protos (ProtoNode ids include qualifiers, so multi-entry names like per-module moduleConfig coexist). Only the instantiation list excludes them - the instances already exist. The selectProto fallback branch is gone; one resolution path. - aop-runtime tests no longer feed the package root as a module (hooks are registered manually there); fixture .egg manifest caches are stale-prone local artifacts, not repo files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restore the pre-refactor design (StandaloneInnerObjectProto) inside the new graph: a provided instance's descriptor is a regular ClassProtoDescriptor whose clazz is the factory `() => obj`, with protoImplType PROVIDED_INNER_OBJECT dispatched through the standard creator registry (the same polymorphism point as COMPATIBLE/INNER_OBJECT proto types). "Constructing" one returns the instance. One uniform channel: the builder feeds every descriptor - hooks and provided alike - through the same vertices, edge resolution, topological sort, createProtoByDescriptor loop and instantiation loop. The sort-output filter and the load unit's separate #innerObjects registration channel are both gone; InnerObjectLoadUnitOptions.innerObjects is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Vestige of the intermediate round where aop-runtime itself was the scanned module; plugin/aop (the actual aop module) re-exports CrosscutAdviceFactory from aop-decorator directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed-plugin fallback
Fourth review round, closing the discovery question:
The enabled-plugin auto-include (resolveModuleReferences + hasEggModule)
was solving a fixture problem with a mechanism. The pre-existing contract
already covers plugins-as-modules on BOTH declaration styles:
- explicit config/module.json: an npm-form reference to the plugin package
(the dal fixture has carried `{"package": "../../../../"}` all along -
that is HOW its plugin module loaded before this PR);
- scan mode: eggModule-declaring packages in app/framework dependencies
(the standalone dal fixture declares `@eggjs/dal-plugin` in deps).
What broke was never the mechanism: the aop/dal/controller egg fixtures
enable plugins whose hooks now ride the module scan, without declaring the
plugin package as a module. Before the refactor that omission was invisible
(hooks were hand-registered). Fix the fixtures to follow the dal precedent
(`{"package": "@eggjs/aop-plugin"}` in module.json) and revert the
discovery machinery: EggModuleLoader is back to app.moduleReferences,
hasEggModule is gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ProvidedInnerObjectProto stored the `() => obj` factory in a field named `clazz` typed EggProtoImplClass (inherited from the old StandaloneInnerObjectProto trick), making constructEggObject read like an instantiation. Rename to `objFactory: () => object` and call it directly - the only casts left sit at the EggPrototypeLifecycleContext boundary, whose `clazz` slot is the carrier, with comments saying so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… compat Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Core tegg semantics with no declaring app: scan discovery would leak host internals into user module.json. One shared class, one line per host. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the config module Review suggestion: the config-domain hook belongs to the config package, and that package should simply BE a module. This kills the last clazz-list registration and with it the whole registerInnerObjectClazzList API. - @eggjs/tegg-config declares eggModule (teggConfig) and hosts the hook in src/lib (host-safe: its egg imports are type-only). Both hosts consume it as a scanned module: standalone via the third built-in framework module reference; the egg host via the framework-dependency channel. - ModuleScanner now takes the RUNTIME framework dir (app.options.framework, what egg/mm actually resolved) over re-deriving from appPkg.egg.framework - the gap that made framework-deps discovery invisible in test harnesses. With it, packages/egg's own dependencies (aop/dal/config plugins) reach every fixture as OPTIONAL modules, promoted by the existing enabled loop; the fixture module.json declarations added earlier are retired. - ModuleHandler.registerInnerObjectClazzList and its buffer are gone - every hook now arrives through the module scan; ReadModule asserts the production shape (business refs exact, framework refs optional). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up from asking whether standalone can reuse ModuleScanner (it can't: the scanner's increment over readModuleReference is egg-only semantics - single frameworkDir + optional marking gated by plugin enablement, and standalone has no plugin system; the shared primitives already live in common-util): - Graph sort only gates optional modules' BUSINESS protos; their inner hooks were instantiated regardless, so a disabled plugin's hooks silently loaded. instantiateInnerObjectLoadUnit now skips unpromoted optional descriptors - symmetric gating. - That gate exposed a long-dormant bug: buildAppGraph's enabled-promotion matched plugin.path (inside the package, through symlinks) against reference paths (real package roots) and never fired. Resolve the real package root before matching. - StandaloneApp switches its hand-rolled path dedupe to the shared ModuleConfigUtil.deduplicateModules (same first-wins semantics, plus duplicate-name conflict detection). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gxkl
force-pushed
the
feat/module-plugin-core
branch
from
facb7f1 to
80b9d49
Compare
Contributor
Author
|
回复 @killagu 这条汇总 review： #6021 (comment) 这条漏回了，补一个逐项说明。当前 head 已更新到
前 8 外的几项也处理了：
本地验证过的关键项包括： |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tegg/standalone/standalone/README.md (1)
28-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the README example to use
StandaloneApptegg/standalone/standalone/README.md:28-30still importsRunnerfrom@eggjs/tegg/standalone, but that entrypoint now exportsStandaloneApp/maininstead. Update the snippet so copy-paste examples don’t break.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/standalone/standalone/README.md` around lines 28 - 30, The README example is using the old standalone entrypoint API, so update the snippet to match the current exports from `@eggjs/tegg/standalone` by replacing Runner/MainRunner usage with StandaloneApp and main. Adjust the example so it reflects the new startup pattern and remains copy-pasteable for readers using the standalone package.
🧹 Nitpick comments (1)
tegg/standalone/standalone/README.md (1)
46-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language specifier to the fenced code block.
The code block at line 48 lacks a language identifier, which triggers markdownlint MD040. Since the content was modified in this PR, consider adding
tsto the fence.📝 Proposed fix
-``` +```ts await main(cwd, {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/standalone/standalone/README.md` around lines 46 - 58, Add a language identifier to the fenced example in the standalone README so it satisfies markdownlint MD040. Update the code block around the `await main(cwd, { ... })` example to use a TypeScript fence, keeping the surrounding `innerObjectHandlers` documentation unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tegg/plugin/config/src/lib/ModuleScanner.ts`:
- Around line 79-86: The `readAndDeduplicateModuleReferences` flow is applying
the app-only `outDir` exclusion too broadly because
`this.readModuleOptions.extraFilePattern` is being mutated before framework
scans. Update `ModuleScanner` so the `!**/dist` exclusion is scoped only to the
app scan rooted at `this.baseDir`, either by cloning `readModuleOptions` for the
app-specific read or by adding the extra pattern only when scanning the app
directory, and keep framework package scans using the original options.
---
Outside diff comments:
In `@tegg/standalone/standalone/README.md`:
- Around line 28-30: The README example is using the old standalone entrypoint
API, so update the snippet to match the current exports from
`@eggjs/tegg/standalone` by replacing Runner/MainRunner usage with StandaloneApp
and main. Adjust the example so it reflects the new startup pattern and remains
copy-pasteable for readers using the standalone package.
---
Nitpick comments:
In `@tegg/standalone/standalone/README.md`:
- Around line 46-58: Add a language identifier to the fenced example in the
standalone README so it satisfies markdownlint MD040. Update the code block
around the `await main(cwd, { ... })` example to use a TypeScript fence, keeping
the surrounding `innerObjectHandlers` documentation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d4032db-6fcc-4e38-b273-9efe747203b3
⛔ Files ignored due to path filters (23)
tegg/core/aop-runtime/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/core-decorator/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/metadata/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/runtime/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/tegg/test/__snapshots__/exports.test.ts.snapis excluded by!**/*.snaptegg/core/tegg/test/__snapshots__/helper.test.ts.snapis excluded by!**/*.snaptegg/core/types/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/plugin/config/test/fixtures/framework-chain/app/node_modules/base-framework/node_modules/base-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-chain/app/node_modules/base-framework/node_modules/shared-module-base/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-chain/app/node_modules/base-framework/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-chain/app/node_modules/chair-framework/node_modules/chair-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-chain/app/node_modules/chair-framework/node_modules/shared-module-chair/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-chain/app/node_modules/chair-framework/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-cycle/app/node_modules/base-framework/node_modules/base-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-cycle/app/node_modules/base-framework/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-cycle/app/node_modules/chair-framework/node_modules/chair-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-cycle/app/node_modules/chair-framework/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-module-json/app/node_modules/chair-framework/config/module.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-module-json/app/node_modules/chair-framework/node_modules/framework-config-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-module-json/app/node_modules/chair-framework/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-module-json/app/node_modules/framework-config-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-same-path/app/node_modules/chair-framework/node_modules/chair-module/package.jsonis excluded by!**/node_modules/**tegg/plugin/config/test/fixtures/framework-same-path/app/node_modules/chair-framework/package.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (147)
tegg/core/aop-decorator/src/CrosscutAdviceFactory.tstegg/core/aop-runtime/package.jsontegg/core/aop-runtime/src/AopContextAdviceRegistry.tstegg/core/aop-runtime/src/AopGraphHookRegistrar.tstegg/core/aop-runtime/src/EggObjectAopHook.tstegg/core/aop-runtime/src/EggPrototypeCrossCutHook.tstegg/core/aop-runtime/src/LoadUnitAopHook.tstegg/core/aop-runtime/src/index.tstegg/core/aop-runtime/test/aop-runtime.test.tstegg/core/common-util/src/ModuleConfig.tstegg/core/common-util/test/ModuleConfig.test.tstegg/core/core-decorator/src/decorator/DefineModuleQualifier.tstegg/core/core-decorator/src/decorator/EggLifecycleProto.tstegg/core/core-decorator/src/decorator/InnerObjectProto.tstegg/core/core-decorator/src/decorator/index.tstegg/core/core-decorator/src/util/PrototypeUtil.tstegg/core/core-decorator/test/decorators.test.tstegg/core/core-decorator/test/fixtures/decators/QualifierCacheService.tstegg/core/core-decorator/test/inner-object-decorators.test.tstegg/core/loader/src/LoaderFactory.tstegg/core/loader/test/LoaderInnerObject.test.tstegg/core/loader/test/fixtures/modules/module-with-inner-object/ControllerHook.tstegg/core/loader/test/fixtures/modules/module-with-inner-object/FetchRouter.tstegg/core/loader/test/fixtures/modules/module-with-inner-object/HelloService.tstegg/core/loader/test/fixtures/modules/module-with-inner-object/package.jsontegg/core/metadata/src/impl/EggInnerObjectPrototypeImpl.tstegg/core/metadata/src/impl/EggPrototypeBuilder.tstegg/core/metadata/src/impl/EggPrototypeImpl.tstegg/core/metadata/src/impl/InjectObjectPrototypeFinder.tstegg/core/metadata/src/impl/index.tstegg/core/metadata/src/model/ModuleDescriptor.tstegg/core/metadata/src/model/ProtoDescriptorHelper.tstegg/core/metadata/src/model/graph/GlobalGraph.tstegg/core/metadata/src/model/graph/ProtoGraphUtils.tstegg/core/metadata/src/model/graph/index.tstegg/core/metadata/test/ModuleDescriptorDumper.test.tstegg/core/runtime/src/impl/EggInnerObjectImpl.tstegg/core/runtime/src/impl/InnerObjectLoadUnit.tstegg/core/runtime/src/impl/InnerObjectLoadUnitBuilder.tstegg/core/runtime/src/impl/InnerObjectLoadUnitInstance.tstegg/core/runtime/src/impl/ProvidedInnerObjectProto.tstegg/core/runtime/src/impl/index.tstegg/core/runtime/test/InnerObjectLoadUnit.test.tstegg/core/test-util/src/LoaderUtil.tstegg/core/types/package.jsontegg/core/types/src/common/ModuleConfig.tstegg/core/types/src/core-decorator/EggLifecycleProto.tstegg/core/types/src/core-decorator/InnerObjectProto.tstegg/core/types/src/core-decorator/Prototype.tstegg/core/types/src/core-decorator/enum/Qualifier.tstegg/core/types/src/core-decorator/index.tstegg/core/types/src/core-decorator/model/EggLifecycleInfo.tstegg/core/types/src/core-decorator/model/index.tstegg/core/types/src/metadata/model/ProtoDescriptor.tstegg/plugin/aop/package.jsontegg/plugin/aop/src/InnerObjects.tstegg/plugin/aop/src/app.tstegg/plugin/aop/src/lib/AopContextHook.tstegg/plugin/aop/test/aop.test.tstegg/plugin/aop/test/fixtures/apps/aop-app/modules/aop-module/Hello.tstegg/plugin/config/package.jsontegg/plugin/config/src/app.tstegg/plugin/config/src/lib/ConfigSourceLoadUnitHook.tstegg/plugin/config/src/lib/ModuleScanner.tstegg/plugin/config/test/ManifestModuleReference.test.tstegg/plugin/config/test/ModuleScanner.test.tstegg/plugin/config/test/ReadModule.test.tstegg/plugin/config/test/fixtures/framework-chain/app/package.jsontegg/plugin/config/test/fixtures/framework-cycle/app/package.jsontegg/plugin/config/test/fixtures/framework-module-json/app/package.jsontegg/plugin/config/test/fixtures/framework-same-path/app/config/module.jsontegg/plugin/config/test/fixtures/framework-same-path/app/package.jsontegg/plugin/dal/package.jsontegg/plugin/dal/src/app.tstegg/plugin/dal/src/app/extend/application.tstegg/plugin/dal/src/lib/DalModuleLoadUnitHook.tstegg/plugin/dal/src/lib/DalTableEggPrototypeHook.tstegg/plugin/dal/src/lib/DataSource.tstegg/plugin/dal/src/lib/MysqlDataSourceManager.tstegg/plugin/dal/src/lib/SqlMapManager.tstegg/plugin/dal/src/lib/TableModelManager.tstegg/plugin/dal/src/lib/TransactionPrototypeHook.tstegg/plugin/dal/test/transaction.test.tstegg/plugin/tegg/package.jsontegg/plugin/tegg/src/app.tstegg/plugin/tegg/src/lib/EggModuleLoader.tstegg/plugin/tegg/src/lib/ModuleConfigLoader.tstegg/plugin/tegg/src/lib/ModuleHandler.tstegg/plugin/tegg/test/BundledAppBoot.test.tstegg/plugin/tegg/test/ManifestCollection.test.tstegg/plugin/tegg/test/ModulePlugin.test.tstegg/plugin/tegg/test/MultiApp.test.tstegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/config.default.tstegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/module.jsontegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/plugin.tstegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/HelloService.tstegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/InnerRegistry.tstegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/PluginHooks.tstegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/package.jsontegg/plugin/tegg/test/fixtures/apps/module-plugin-app/package.jsontegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/CounterService.tstegg/plugin/tegg/test/lib/EggModuleLoader.test.tstegg/plugin/tegg/test/lib/ModuleHandler.test.tstegg/standalone/standalone/README.mdtegg/standalone/standalone/package.jsontegg/standalone/standalone/src/ConfigSourceLoadUnitHook.tstegg/standalone/standalone/src/EggModuleLoader.tstegg/standalone/standalone/src/Runner.tstegg/standalone/standalone/src/StandaloneApp.tstegg/standalone/standalone/src/StandaloneInnerObject.tstegg/standalone/standalone/src/StandaloneInnerObjectProto.tstegg/standalone/standalone/src/StandaloneLoadUnit.tstegg/standalone/standalone/src/index.tstegg/standalone/standalone/src/main.tstegg/standalone/standalone/test/ModulePlugin.test.tstegg/standalone/standalone/test/fixtures/dal-manager-inject/foo.tstegg/standalone/standalone/test/fixtures/dal-manager-inject/package.jsontegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/Foo.tstegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/FooEggContextHook.tstegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/package.jsontegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/Foo.tstegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/FooEggObjectHook.tstegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/package.jsontegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/Foo.tstegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/FooEggPrototypeHook.tstegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/package.jsontegg/standalone/standalone/test/fixtures/inner-object-proto/foo.tstegg/standalone/standalone/test/fixtures/inner-object-proto/innerBar.tstegg/standalone/standalone/test/fixtures/inner-object-proto/package.jsontegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/foo.tstegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/innerBar.tstegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/package.jsontegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/Foo.tstegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/FooLoadUnitInstanceHook.tstegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/package.jsontegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Foo.tstegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/FooLoadUnitHook.tstegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Runner.tstegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/package.jsontegg/standalone/standalone/test/fixtures/logger-option/foo.tstegg/standalone/standalone/test/fixtures/logger-option/package.jsontegg/standalone/standalone/test/index.test.tstegg/standalone/standalone/tsdown.config.tstools/egg-bundler/src/lib/ManifestLoader.tswiki/concepts/tegg-module-plugin.mdwiki/index.mdwiki/log.md
💤 Files with no reviewable changes (9)
- tegg/standalone/standalone/src/StandaloneInnerObjectProto.ts
- tegg/standalone/standalone/src/StandaloneLoadUnit.ts
- tegg/plugin/dal/package.json
- tegg/plugin/dal/src/app/extend/application.ts
- tegg/plugin/tegg/package.json
- tegg/standalone/standalone/src/ConfigSourceLoadUnitHook.ts
- tegg/standalone/standalone/src/Runner.ts
- tegg/plugin/dal/src/app.ts
- tegg/standalone/standalone/src/StandaloneInnerObject.ts
✅ Files skipped from review due to trivial changes (30)
- tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/package.json
- tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/package.json
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/package.json
- tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/package.json
- tegg/plugin/config/test/fixtures/framework-cycle/app/package.json
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/ControllerHook.ts
- tegg/core/types/src/core-decorator/model/index.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/module.json
- tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/package.json
- tegg/core/metadata/src/model/graph/index.ts
- tegg/standalone/standalone/test/fixtures/inner-object-proto/package.json
- wiki/index.md
- tegg/core/metadata/test/ModuleDescriptorDumper.test.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/package.json
- tegg/core/aop-runtime/src/index.ts
- tegg/plugin/config/src/lib/ConfigSourceLoadUnitHook.ts
- tegg/plugin/config/test/fixtures/framework-same-path/app/package.json
- tegg/plugin/aop/package.json
- tegg/standalone/standalone/test/fixtures/dal-manager-inject/package.json
- tegg/core/aop-decorator/src/CrosscutAdviceFactory.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/FetchRouter.ts
- tegg/core/types/src/core-decorator/EggLifecycleProto.ts
- tegg/plugin/aop/src/InnerObjects.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/plugin.ts
- tegg/plugin/tegg/test/BundledAppBoot.test.ts
- tegg/plugin/config/test/ManifestModuleReference.test.ts
- tegg/core/metadata/src/impl/index.ts
- tegg/plugin/config/test/fixtures/framework-same-path/app/config/module.json
- tegg/standalone/standalone/test/fixtures/logger-option/package.json
- wiki/log.md
🚧 Files skipped from review as they are similar to previous changes (102)
- tegg/core/core-decorator/src/decorator/DefineModuleQualifier.ts
- tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/foo.ts
- tegg/core/types/src/core-decorator/index.ts
- tegg/core/types/src/core-decorator/enum/Qualifier.ts
- tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/package.json
- tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/Foo.ts
- tegg/standalone/standalone/tsdown.config.ts
- tegg/core/types/src/core-decorator/InnerObjectProto.ts
- tegg/standalone/standalone/test/ModulePlugin.test.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/package.json
- tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/FooEggContextHook.ts
- tegg/core/runtime/src/impl/index.ts
- tegg/core/types/src/core-decorator/model/EggLifecycleInfo.ts
- tegg/core/types/src/common/ModuleConfig.ts
- tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/innerBar.ts
- tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/FooEggPrototypeHook.ts
- tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/FooLoadUnitInstanceHook.ts
- tegg/core/core-decorator/src/decorator/InnerObjectProto.ts
- tegg/standalone/standalone/test/fixtures/inner-object-proto/foo.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Foo.ts
- tools/egg-bundler/src/lib/ManifestLoader.ts
- tegg/plugin/dal/test/transaction.test.ts
- tegg/plugin/config/package.json
- tegg/core/types/src/metadata/model/ProtoDescriptor.ts
- tegg/plugin/tegg/test/MultiApp.test.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/HelloService.ts
- tegg/core/aop-runtime/src/AopContextAdviceRegistry.ts
- tegg/core/aop-runtime/src/EggObjectAopHook.ts
- tegg/plugin/config/test/ReadModule.test.ts
- tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/Foo.ts
- tegg/plugin/config/test/fixtures/framework-chain/app/package.json
- tegg/plugin/aop/test/fixtures/apps/aop-app/modules/aop-module/Hello.ts
- tegg/core/types/src/core-decorator/Prototype.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/FooLoadUnitHook.ts
- tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/Foo.ts
- tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/FooEggObjectHook.ts
- tegg/standalone/standalone/test/fixtures/logger-option/foo.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Runner.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/HelloService.ts
- tegg/plugin/tegg/test/ModulePlugin.test.ts
- tegg/core/metadata/src/impl/EggPrototypeImpl.ts
- tegg/plugin/tegg/test/lib/EggModuleLoader.test.ts
- tegg/standalone/standalone/test/fixtures/inner-object-proto/innerBar.ts
- tegg/core/loader/test/LoaderInnerObject.test.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/package.json
- tegg/standalone/standalone/test/fixtures/dal-manager-inject/foo.ts
- tegg/plugin/aop/test/aop.test.ts
- tegg/core/core-decorator/test/fixtures/decators/QualifierCacheService.ts
- tegg/core/metadata/src/impl/EggInnerObjectPrototypeImpl.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/config.default.ts
- tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/Foo.ts
- tegg/core/core-decorator/test/decorators.test.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/PluginHooks.ts
- tegg/standalone/standalone/package.json
- tegg/standalone/standalone/src/index.ts
- tegg/core/aop-runtime/src/AopGraphHookRegistrar.ts
- tegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/CounterService.ts
- tegg/core/metadata/src/model/ProtoDescriptorHelper.ts
- tegg/plugin/config/test/fixtures/framework-module-json/app/package.json
- tegg/plugin/tegg/test/lib/ModuleHandler.test.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/InnerRegistry.ts
- tegg/standalone/standalone/src/main.ts
- tegg/plugin/dal/src/lib/TransactionPrototypeHook.ts
- tegg/plugin/config/src/app.ts
- tegg/core/runtime/src/impl/InnerObjectLoadUnit.ts
- tegg/core/test-util/src/LoaderUtil.ts
- tegg/plugin/dal/src/lib/DataSource.ts
- tegg/core/core-decorator/src/decorator/EggLifecycleProto.ts
- tegg/plugin/dal/src/lib/MysqlDataSourceManager.ts
- tegg/core/core-decorator/src/decorator/index.ts
- tegg/plugin/tegg/test/ManifestCollection.test.ts
- tegg/core/metadata/src/model/graph/ProtoGraphUtils.ts
- tegg/core/metadata/src/model/graph/GlobalGraph.ts
- tegg/plugin/aop/src/app.ts
- tegg/plugin/dal/src/lib/DalTableEggPrototypeHook.ts
- tegg/core/aop-runtime/test/aop-runtime.test.ts
- tegg/core/aop-runtime/src/EggPrototypeCrossCutHook.ts
- tegg/core/types/package.json
- tegg/core/common-util/test/ModuleConfig.test.ts
- tegg/core/core-decorator/test/inner-object-decorators.test.ts
- tegg/plugin/config/test/ModuleScanner.test.ts
- tegg/core/aop-runtime/package.json
- tegg/plugin/dal/src/lib/TableModelManager.ts
- tegg/plugin/dal/src/lib/DalModuleLoadUnitHook.ts
- tegg/core/metadata/src/impl/EggPrototypeBuilder.ts
- tegg/core/runtime/src/impl/InnerObjectLoadUnitBuilder.ts
- tegg/plugin/tegg/src/lib/ModuleConfigLoader.ts
- tegg/core/metadata/src/impl/InjectObjectPrototypeFinder.ts
- tegg/plugin/tegg/src/app.ts
- tegg/core/runtime/test/InnerObjectLoadUnit.test.ts
- tegg/core/loader/src/LoaderFactory.ts
- tegg/core/runtime/src/impl/InnerObjectLoadUnitInstance.ts
- tegg/core/runtime/src/impl/ProvidedInnerObjectProto.ts
- tegg/plugin/dal/src/lib/SqlMapManager.ts
- tegg/plugin/aop/src/lib/AopContextHook.ts
- tegg/standalone/standalone/test/index.test.ts
- tegg/core/metadata/src/model/ModuleDescriptor.ts
- tegg/standalone/standalone/src/EggModuleLoader.ts
- tegg/plugin/tegg/src/lib/EggModuleLoader.ts
- tegg/core/runtime/src/impl/EggInnerObjectImpl.ts
- tegg/standalone/standalone/src/StandaloneApp.ts
- tegg/plugin/tegg/src/lib/ModuleHandler.ts
Comment on lines
+79
to
+86
| private readAndDeduplicateModuleReferences(baseDir: string, cwd?: string): readonly ModuleReference[] { | ||
| return ModuleConfigUtil.deduplicateModules( | ||
| ModuleConfigUtil.readModuleReference(baseDir, { | ||
| ...this.readModuleOptions, | ||
| ...(cwd ? { cwd } : {}), | ||
| }), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how readModuleReference consumes extraFilePattern + cwd to confirm
# whether an app outDir exclusion (e.g. !**/dist) can exclude framework
# packages scanned from node_modules.
fd -a 'ModuleConfig*.ts' tegg/core/common-util/src | while read -r f; do
echo "== $f =="
ast-grep outline "$f" --items all
done
rg -nP -C4 '\b(extraFilePattern|readModuleReference|cwd)\b' tegg/core/common-util/srcRepository: eggjs/egg
Length of output: 5268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ModuleScanner.ts =="
ast-grep outline tegg/plugin/config/src/lib/ModuleScanner.ts --items all
echo
echo "== nearby lines =="
cat -n tegg/plugin/config/src/lib/ModuleScanner.ts | sed -n '1,220p'
echo
echo "== scanModuleReferences callers =="
rg -n -C 4 'scanModuleReferences|extraFilePattern|outDir|readModuleOptions' tegg/plugin/config/src tegg -g '*.ts'Repository: eggjs/egg
Length of output: 30326
Scope the outDir exclusion to the app scan only.
readModuleOptions.extraFilePattern is mutated before ModuleScanner scans framework directories, so !**/dist also applies to published framework packages. That can hide framework module plugins shipped only from compiled output; clone the options for the app scan or add the exclusion only for this.baseDir.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tegg/plugin/config/src/lib/ModuleScanner.ts` around lines 79 - 86, The
`readAndDeduplicateModuleReferences` flow is applying the app-only `outDir`
exclusion too broadly because `this.readModuleOptions.extraFilePattern` is being
mutated before framework scans. Update `ModuleScanner` so the `!**/dist`
exclusion is scoped only to the app scan rooted at `this.baseDir`, either by
cloning `readModuleOptions` for the app-specific read or by adding the extra
pattern only when scanning the app directory, and keep framework package scans
using the original options.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Tegg's framework-level extensions (AOP, DAL, controller registration, config source, ...) all rely on the HOST hand-registering lifecycle hooks in its boot code. A module cannot declare "run this when a load unit is created" by itself, so:
Runner;This PR ports the declarative module plugin design from eggjs/tegg#325 (merged to the stale
standalone-nextbranch, never reached master) to thenextarchitecture, and completes the two parts #325 left open: egg-host wiring (inner object protos used to be silently ignored in app mode) and bootstrapping the built-in hooks themselves (the#handleCompatibilityTODO).Design
A plain eggModule package can now provide framework extensions declaratively:
@InnerObjectProto— framework inner object (a SingletonProto withEGG_INNER_OBJECT_PROTO_IMPL_TYPE), diverted by the loader intoModuleDescriptor.innerObjectClazzList, never into business load units.@EggLifecycleProtofive typed variants (LoadUnit/LoadUnitInstance/EggPrototype/EggObject/EggContext) — a DI-capable hook object, auto-registered into the matching scope-aware LifecycleUtil and deregistered symmetrically.Two-phase ordering (the load-bearing constraint).
GlobalGraph.create()only adds nodes;build()adds inject edges and consumesregisterBuildHookhooks once at its end (late registration is silently lost). Both hosts now boot as:GlobalGraph.create(nodes only)InnerObjectLoadUnit(own topologically sorted proto graph, cycle detection, hard error on missing non-optional deps) — hooks register here, including graph build hooks from@LifecyclePostInject(seeAopGraphHookRegistrar)build()/sort()→ business load units → business instancesCommits
feat(core): add module plugin core mechanismEggInnerObjectPrototypeImpl,InjectObjectPrototypeFinder/ProtoGraphUtilsextracted with a proto-name index replacing the O(n·m·n) scan) + runtime (EggInnerObjectImplruns only decorator-declared self lifecycle; host-agnosticInnerObjectLoadUnit(Builder/Instance)) + loader diversion (manifestgetDecoratedFilescovers the new list so bundle mode still re-imports those files)feat(standalone): two-phase StandaloneApp with module plugin supportRunnerrenamed toStandaloneApp(no alias;main()/preLoad()unchanged). Explicit boot phases,frameworkDepsoption, bundle-mode manifest/loaderFS consumption +StandaloneApp.loadMetadata(), tegg#325 test suite portedfeat(tegg-plugin): instantiate InnerObjectLoadUnit in app modeModuleHandler.init()phases viaEggModuleLoader.initGraph()/load()split; the same module plugin package behaves identically under both hostsfeat(tegg): convert built-in framework hooks to module pluginsAopGraphHookRegistrarfor the crossCut/pointCut build hooks), DAL (constructor args →@Injectof host-providedmoduleConfigs/runtimeConfig/loggerinner objects, PRIVATE on the egg host), ConfigSource; vestigialLoadUnitMultiInstanceProtoHookregistration dropped (emptypreCreate, unconsumed static set); builder dedupes by class since a package may be both hard-fed and scanned as an eggModule (e.g.@eggjs/dal-plugin)docs(wiki): record tegg module plugin architectureTest evidence
InnerObjectLoadUnitintegration incl. decorator-only-lifecycle semantics / auto register+deregister / cycle detection / missing-dep hard error (4), standalone module plugin suite ported from tegg#325 — five lifecycle proto types + inner object PUBLIC/PRIVATE semantics (7), app-mode module plugin fixture (2).MultiAppisolation green; full-repo run has zero failures related to this change (remaining ones verified as pre-existing/env: dns-cache needs a freshut install, multipart fails on cleannexttoo, onerror/development are flaky-on-rerun).Notes
git merge-treeclean; combined-tree run of both test suites green).🤖 Generated with Claude Code
Summary by CodeRabbit