feat(tegg): declarative module plugin mechanism#6021
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds inner-object and lifecycle decorators, updates metadata and graph resolution, introduces runtime/load-unit support, migrates AOP/config/DAL and boot wiring, and replaces the standalone Runner with StandaloneApp. Tests, fixtures, and docs are updated to match the new module-plugin flow. 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 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: |
5c42a6d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://618b1775.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-module-plugin-core.egg-cci.pages.dev |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #6021 +/- ##
==========================================
+ Coverage 81.98% 82.19% +0.20%
==========================================
Files 678 686 +8
Lines 20838 21195 +357
Branches 4154 4224 +70
==========================================
+ Hits 17085 17422 +337
- Misses 3235 3256 +21
+ Partials 518 517 -1 ☔ 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: |
5c42a6d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8bd664a5.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-module-plugin-core.egg-v3.pages.dev |
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.
| import { crossCutGraphHook } from './CrossCutGraphHook.js'; | ||
| import { pointCutGraphHook } from './PointCutGraphHook.js'; |
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.
| 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.
There was a problem hiding this comment.
不采纳这里的 .ts import 建议。tegg 源码当前按 ESM 输出规范使用 .js import specifier,同包其它 TS 源码也是这个模式;改成 .ts 反而会和现有构建/类型检查约定不一致。
| import { AopGraphHookRegistrar } from './AopGraphHookRegistrar.js'; | ||
| import { EggObjectAopHook } from './EggObjectAopHook.js'; | ||
| import { EggPrototypeCrossCutHook } from './EggPrototypeCrossCutHook.js'; | ||
| import { LoadUnitAopHook } from './LoadUnitAopHook.js'; |
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.
| 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.
There was a problem hiding this comment.
不改,原因同上:tegg TS 源码使用 .js import specifier 才符合当前 ESM 输出约定。这个 thread 现在也已经是 outdated。
| export * from './AopGraphHookRegistrar.js'; | ||
| export * from './AopInnerObjectClazzList.js'; |
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.
| 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.
There was a problem hiding this comment.
不改,原因同上:这里的 export specifier 也应保持 .js,和 tegg 现有 ESM 源码约定一致。这个 thread 现在已 outdated。
| ctx.destroy(lifecycle).catch((e) => { | ||
| e.message = `[tegg/standalone] destroy tegg context failed: ${e.message}`; | ||
| console.warn(e); | ||
| }); |
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.
| 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.
There was a problem hiding this comment.
已按这个方向处理:destroy catch 里不再直接假设 caught value 一定是 Error;会先做 Error 判断,非 Error 用 String(e) 兜底格式化,避免 primitive throw 时二次抛错。这个 thread 目前已 outdated。
| app.destroy().catch((e) => { | ||
| e.message = `[tegg/standalone] destroy tegg failed: ${e.message}`; | ||
| console.warn(e); | ||
| }); |
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.
| 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.
There was a problem hiding this comment.
已处理:main 成功/失败路径里都会等待 app destroy,并且 destroy catch 对非 Error 值做 String(e) 兜底,不再直接改 e.message。这个 thread 目前已 outdated。
5001fb6 to
79594a6
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a declarative “module plugin” mechanism for Tegg so eggModule packages can contribute framework inner objects and lifecycle hooks without host boot-code registration, and updates both the standalone host and egg host to boot in a two-phase order that guarantees graph build hooks are registered before GlobalGraph.build() consumes them.
Changes:
- Add
@InnerObjectProto/@EggLifecycleProto(five lifecycle variants) and runtime support via anInnerObjectLoadUnitinstantiated before business load units. - Refactor standalone host: rename
Runner→StandaloneApp, add bundle-manifest support, and update/expand module-plugin test coverage. - Update egg host boot wiring (
ModuleHandler/EggModuleLoader) to instantiate theInnerObjectLoadUnitbefore loading business units; migrate built-in AOP/DAL/ConfigSource hooks to module-plugin form; document the architecture inwiki/.
Reviewed changes
Copilot reviewed 127 out of 127 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wiki/log.md | Logs the new module-plugin architecture work and host ordering constraints. |
| wiki/index.md | Adds an index entry for the new module-plugin concept page. |
| wiki/concepts/tegg-module-plugin.md | Documents the declarative module-plugin design and two-phase boot ordering. |
| tegg/standalone/standalone/tsdown.config.ts | Configures unused-check ignore list for framework module-plugin deps. |
| tegg/standalone/standalone/test/ModulePlugin.test.ts | Adds standalone integration coverage for lifecycle protos + inner objects. |
| tegg/standalone/standalone/test/index.test.ts | Updates tests for StandaloneApp, adds logger option and DAL cleanup coverage. |
| tegg/standalone/standalone/test/fixtures/logger-option/package.json | Adds a fixture eggModule for logger injection tests. |
| tegg/standalone/standalone/test/fixtures/logger-option/foo.ts | Fixture runner validating injected logger inner object. |
| tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Runner.ts | Fixture validating LoadUnit lifecycle proto timing + DI. |
| tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/package.json | Adds eggModule metadata for the lifecycle-proto fixture. |
| tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/FooLoadUnitHook.ts | Fixture hook that mutates business load unit protos during preCreate. |
| tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Foo.ts | Fixture inner object proto used by lifecycle hook DI. |
| tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/package.json | Adds eggModule metadata for LoadUnitInstance lifecycle fixture. |
| tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/FooLoadUnitInstanceHook.ts | Fixture LoadUnitInstance hook mutating created objects. |
| tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/Foo.ts | Fixture runner exposing mutation performed by instance hook. |
| tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/package.json | Adds eggModule metadata for private-inner-object injection failure test. |
| tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/innerBar.ts | Fixture private inner object proto. |
| tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/foo.ts | Fixture business proto that incorrectly injects private inner object. |
| tegg/standalone/standalone/test/fixtures/inner-object-proto/package.json | Adds eggModule metadata for inner-object injection fixture. |
| tegg/standalone/standalone/test/fixtures/inner-object-proto/innerBar.ts | Fixture PUBLIC inner object proto with DI dependency on another inner object. |
| tegg/standalone/standalone/test/fixtures/inner-object-proto/foo.ts | Fixture business proto injecting a PUBLIC inner object. |
| tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/package.json | Adds eggModule metadata for EggPrototype lifecycle fixture. |
| tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/FooEggPrototypeHook.ts | Fixture prototype hook writing metadata from prototype creation. |
| tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/Foo.ts | Fixture runner reading data set by prototype hook. |
| tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/package.json | Adds eggModule metadata for EggObject lifecycle fixture. |
| tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/FooEggObjectHook.ts | Fixture object hook mutating created object. |
| tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/Foo.ts | Fixture runner reading mutation from object hook. |
| tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/package.json | Adds eggModule metadata for EggContext lifecycle fixture. |
| tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/FooEggContextHook.ts | Fixture context hook initializing context state. |
| tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/Foo.ts | Fixture runner reading ctx state via ContextHandler. |
| tegg/standalone/standalone/test/fixtures/dal-manager-inject/package.json | Adds eggModule metadata for DAL manager injection fixture. |
| tegg/standalone/standalone/test/fixtures/dal-manager-inject/foo.ts | Fixture pinning PUBLIC mysqlDataSourceManager injection surface. |
| tegg/standalone/standalone/src/StandaloneLoadUnit.ts | Removes legacy standalone “inner object as a load unit” implementation. |
| tegg/standalone/standalone/src/StandaloneInnerObjectProto.ts | Removes legacy standalone inner-object prototype implementation. |
| tegg/standalone/standalone/src/StandaloneInnerObject.ts | Removes legacy standalone inner-object EggObject implementation. |
| tegg/standalone/standalone/src/StandaloneApp.ts | New standalone host implementing two-phase boot with InnerObjectLoadUnit. |
| tegg/standalone/standalone/src/Runner.ts | Removes old standalone host (replaced by StandaloneApp). |
| tegg/standalone/standalone/src/main.ts | Updates main entry to use StandaloneApp and introduces appMain. |
| tegg/standalone/standalone/src/index.ts | Updates exports to expose StandaloneApp instead of removed legacy types. |
| tegg/standalone/standalone/src/EggModuleLoader.ts | Adds manifest/loaderFS support and exposes moduleDescriptors for inner unit build. |
| tegg/standalone/standalone/src/ConfigSourceLoadUnitHook.ts | Removes standalone-specific ConfigSource hook (now host-agnostic module plugin). |
| tegg/standalone/standalone/package.json | Switches framework deps to module-plugin packages and adds loader-fs/tegg-config. |
| tegg/plugin/tegg/test/ModulePlugin.test.ts | Adds app-mode integration coverage for module-plugin lifecycle protos + inner objects. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/package.json | Adds ESM test app fixture root. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/PluginHooks.ts | Fixture module plugin hooks (LoadUnit + EggObject lifecycle protos). |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/package.json | Adds eggModule metadata for the fixture module. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/InnerRegistry.ts | Fixture inner objects with PRIVATE/PUBLIC semantics and DI. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/HelloService.ts | Fixture business singleton injecting a PUBLIC inner object. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/plugin.ts | Enables teggConfig and tegg plugin for app-mode fixture. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/module.json | Declares the fixture module path for scanning. |
| tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/config.default.ts | Minimal keys config for egg mock app boot. |
| tegg/plugin/tegg/test/BundledAppBoot.test.ts | Extends timeout to accommodate additional boots/manifest generation. |
| tegg/plugin/tegg/src/lib/ModuleHandler.ts | Instantiates InnerObjectLoadUnit before business units; reverses teardown ordering. |
| tegg/plugin/tegg/src/lib/ModuleConfigLoader.ts | Adds bundle-mode tolerance when module dirs don’t exist on disk. |
| tegg/plugin/tegg/src/lib/EggModuleLoader.ts | Splits initGraph() vs load(), adds manifest collection changes, promotes enabled plugins reliably. |
| tegg/plugin/tegg/src/app.ts | Removes imperative registration for config-source + multi-instance hook (now module plugins / vestigial removed). |
| tegg/plugin/tegg/package.json | Removes export for deleted ConfigSourceLoadUnitHook. |
| tegg/plugin/dal/src/lib/TransactionPrototypeHook.ts | Converts DAL transaction hook to module-plugin lifecycle proto + DI-injected deps. |
| tegg/plugin/dal/src/lib/MysqlDataSourceManagerObject.ts | Adds PUBLIC inner-object wrapper exposing DAL manager to business modules. |
| tegg/plugin/dal/src/lib/DalTableEggPrototypeHook.ts | Converts DAL table hook to module-plugin lifecycle proto with DI logger. |
| tegg/plugin/dal/src/lib/DalModuleLoadUnitHook.ts | Converts DAL load-unit hook to module-plugin + adds @LifecycleDestroy cleanup. |
| tegg/plugin/dal/src/index.ts | Exports the new MysqlDataSourceManagerObject. |
| tegg/plugin/dal/src/app.ts | Removes imperative hook registration; retains egg-side manager cleanup in beforeClose. |
| tegg/plugin/dal/package.json | Adds export mapping for MysqlDataSourceManagerObject. |
| tegg/plugin/config/test/ReadModule.test.ts | Updates expectations to account for optional framework module references. |
| tegg/plugin/config/src/lib/ModuleScanner.ts | Resolves framework dir via getFrameworkPath and scans framework modules as optional. |
| tegg/plugin/config/src/lib/ConfigSourceLoadUnitHook.ts | Converts config-source hook into host-agnostic module-plugin lifecycle proto. |
| tegg/plugin/config/src/app.ts | Adds bundle-mode tolerance for missing external module dirs/configs. |
| tegg/plugin/config/package.json | Marks tegg-config as an eggModule and exports ConfigSourceLoadUnitHook. |
| tegg/plugin/aop/test/aop.test.ts | Increases timeout to avoid slow-runner failures. |
| tegg/plugin/aop/src/InnerObjects.ts | Re-exports AOP module-plugin classes for scanning under the aop plugin eggModule. |
| tegg/plugin/aop/src/app.ts | Removes imperative hook registration; documents which hooks remain host-registered. |
| tegg/plugin/aop/package.json | Marks aop plugin as an eggModule and exports InnerObjects entry. |
| tegg/core/types/test/snapshots/index.test.ts.snap | Snapshot updates for new exported constants/types. |
| tegg/core/types/src/metadata/model/ProtoDescriptor.ts | Extends InjectObjectDescriptor with optional?: boolean. |
| tegg/core/types/src/core-decorator/Prototype.ts | Adds EGG_INNER_OBJECT_PROTO_IMPL_TYPE constant. |
| tegg/core/types/src/core-decorator/model/index.ts | Exposes EggLifecycleInfo type. |
| tegg/core/types/src/core-decorator/model/EggLifecycleInfo.ts | Adds EggLifecycleInfo model type. |
| tegg/core/types/src/core-decorator/InnerObjectProto.ts | Adds InnerObjectProto params type. |
| tegg/core/types/src/core-decorator/index.ts | Exports EggLifecycleProto + InnerObjectProto types. |
| tegg/core/types/src/core-decorator/EggLifecycleProto.ts | Adds EggLifecycleProto param types. |
| tegg/core/types/package.json | Adds export map entries for new core-decorator typings. |
| tegg/core/test-util/src/LoaderUtil.ts | Reuses production loader classification to match inner-object diversion behavior in tests. |
| tegg/core/tegg/test/snapshots/helper.test.ts.snap | Snapshot updates for new exports. |
| tegg/core/tegg/test/snapshots/exports.test.ts.snap | Snapshot updates for new exports/decorators. |
| tegg/core/tegg/test/snapshots/dal.test.ts.snap | Snapshot updates for DAL exports. |
| tegg/core/runtime/test/InnerObjectLoadUnit.test.ts | Adds integration tests for inner objects, DI wiring, lifecycle proto registration, and error cases. |
| tegg/core/runtime/test/snapshots/index.test.ts.snap | Snapshot updates for new runtime exports. |
| tegg/core/runtime/src/impl/ProvidedInnerObjectProto.ts | Adds proto/object for host-provided inner objects (provided-instance factory). |
| tegg/core/runtime/src/impl/InnerObjectLoadUnitInstance.ts | Implements instantiation + auto register/deregister of lifecycle protos by type. |
| tegg/core/runtime/src/impl/InnerObjectLoadUnitBuilder.ts | Builds a dedicated inner-object proto graph (toposort/cycle detect/missing dep errors). |
| tegg/core/runtime/src/impl/InnerObjectLoadUnit.ts | Introduces the host-agnostic InnerObjectLoadUnit type. |
| tegg/core/runtime/src/impl/index.ts | Exposes new inner-object runtime types and EggInnerObjectImpl. |
| tegg/core/runtime/src/impl/EggInnerObjectImpl.ts | Adds inner-object EggObject implementation that only runs decorator-declared self lifecycle. |
| tegg/core/metadata/test/ModuleDescriptorDumper.test.ts | Updates dumper tests to include innerObjectClazzList. |
| tegg/core/metadata/test/snapshots/index.test.ts.snap | Snapshot updates for new metadata exports. |
| tegg/core/metadata/src/model/ProtoDescriptorHelper.ts | Supports define-vs-instance module/unit fields for diverted protos. |
| tegg/core/metadata/src/model/ModuleDescriptor.ts | Adds innerObjectClazzList and fixes JSON dumping on Windows path escaping. |
| tegg/core/metadata/src/model/graph/ProtoGraphUtils.ts | Extracts proto dependency resolution utilities + name index for perf. |
| tegg/core/metadata/src/model/graph/index.ts | Exports ProtoGraphUtils. |
| tegg/core/metadata/src/model/graph/GlobalGraph.ts | Uses ProtoGraphUtils + lazy name index to avoid O(n·m·n) scanning. |
| tegg/core/metadata/src/impl/InjectObjectPrototypeFinder.ts | Extracts inject-proto selection logic for reuse and optional handling. |
| tegg/core/metadata/src/impl/index.ts | Exports EggInnerObjectPrototypeImpl + InjectObjectPrototypeFinder. |
| tegg/core/metadata/src/impl/EggPrototypeBuilder.ts | Refactors prototype building to use InjectObjectPrototypeFinder. |
| tegg/core/metadata/src/impl/EggInnerObjectPrototypeImpl.ts | Adds proto impl type for inner objects built from metadata descriptors. |
| tegg/core/loader/test/LoaderInnerObject.test.ts | Adds coverage that inner-object classes are diverted and included in manifest decorated files. |
| tegg/core/loader/test/fixtures/modules/module-with-inner-object/package.json | Adds test module fixture with eggModule metadata. |
| tegg/core/loader/test/fixtures/modules/module-with-inner-object/HelloService.ts | Fixture business singleton proto. |
| tegg/core/loader/test/fixtures/modules/module-with-inner-object/FetchRouter.ts | Fixture inner object proto. |
| tegg/core/loader/test/fixtures/modules/module-with-inner-object/ControllerHook.ts | Fixture lifecycle proto injecting inner object. |
| tegg/core/loader/src/LoaderUtil.ts | Tightens glob excludes to also omit test/** and coverage/**. |
| tegg/core/loader/src/LoaderFactory.ts | Diverts inner-object protos into innerObjectClazzList ahead of business clazzList. |
| tegg/core/core-decorator/test/inner-object-decorators.test.ts | Adds tests for InnerObjectProto/EggLifecycleProto metadata semantics. |
| tegg/core/core-decorator/test/snapshots/index.test.ts.snap | Snapshot updates for new decorator exports. |
| tegg/core/core-decorator/src/util/PrototypeUtil.ts | Adds metadata flags for inner objects + lifecycle protos and stores lifecycle type metadata. |
| tegg/core/core-decorator/src/decorator/InnerObjectProto.ts | Adds InnerObjectProto decorator mapping to inner-object proto impl type + flagging. |
| tegg/core/core-decorator/src/decorator/index.ts | Exports new decorators. |
| tegg/core/core-decorator/src/decorator/EggLifecycleProto.ts | Adds EggLifecycleProto decorator + five typed variants. |
| tegg/core/aop-runtime/test/aop-runtime.test.ts | Updates tests to inject CrosscutAdviceFactory via DI field instead of constructor. |
| tegg/core/aop-runtime/test/snapshots/index.test.ts.snap | Snapshot updates for new AOP runtime export. |
| tegg/core/aop-runtime/src/LoadUnitAopHook.ts | Converts to module-plugin lifecycle proto with injected CrosscutAdviceFactory. |
| tegg/core/aop-runtime/src/index.ts | Exports AopGraphHookRegistrar. |
| tegg/core/aop-runtime/src/EggPrototypeCrossCutHook.ts | Converts to module-plugin lifecycle proto with injected CrosscutAdviceFactory. |
| tegg/core/aop-runtime/src/EggObjectAopHook.ts | Converts to module-plugin EggObject lifecycle proto. |
| tegg/core/aop-runtime/src/AopGraphHookRegistrar.ts | Adds declarative registration of GlobalGraph build hooks via @LifecyclePostInject. |
| tegg/core/aop-runtime/package.json | Adds lifecycle dep and removes eggModule declaration from aop-runtime package. |
| tegg/core/aop-decorator/src/CrosscutAdviceFactory.ts | Marks CrosscutAdviceFactory as an inner-object proto for module-plugin instantiation. |
| export async function preLoad(cwd: string, dependencies?: StandaloneAppOptions['dependencies']): Promise<void> { | ||
| try { | ||
| await Runner.preLoad(cwd, dependencies); | ||
| await StandaloneApp.preLoad(cwd, dependencies); | ||
| } catch (e) { |
There was a problem hiding this comment.
已处理:preLoad() 增加并透传 frameworkDeps,和 StandaloneApp.preLoad() 保持一致,避免 framework module plugins 在 preload metadata 中缺失。这个 thread 目前已 outdated。
| `"name": ${JSON.stringify(clazz.name)},` + | ||
| (PrototypeUtil.getFilePath(clazz) | ||
| ? `"filePath": "${path.relative(moduleDescriptor.unitPath, PrototypeUtil.getFilePath(clazz)!)}"` | ||
| ? `"filePath": ${JSON.stringify(path.relative(moduleDescriptor.unitPath, PrototypeUtil.getFilePath(clazz)!))}` | ||
| : '') + | ||
| '}' |
There was a problem hiding this comment.
已处理:stringifyClazz() 改成先组装普通对象,再 JSON.stringify,避免缺 filePath 时生成带尾逗号的非法 JSON。这个 thread 目前已 outdated。
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tegg/core/loader/src/LoaderFactory.ts (1)
61-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGive each module its own
multiInstanceClazzList. Declaring the array outside the loop makes everyModuleDescriptorshare the same list, so later modules mutate earlier descriptors andGlobalGraph.create()attributes multi-instance classes to the wrong module.🤖 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/core/loader/src/LoaderFactory.ts` around lines 61 - 62, Each ModuleDescriptor is currently sharing the same multiInstanceClazzList, so later iterations can mutate earlier descriptors and confuse GlobalGraph.create() attribution. Move the multiInstanceClazzList initialization so it is created per module inside the LoaderFactory logic that builds each ModuleDescriptor, and make sure each descriptor receives its own fresh array before pushing multi-instance classes.tegg/standalone/standalone/src/main.ts (1)
10-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winForward
frameworkDepsthroughpreLoad
preLoadonly acceptsdependencies, so callers can’t warm the same framework module set thatmain/StandaloneApp.preLoadscan at boot. AddframeworkDepshere and pass it through.🤖 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/src/main.ts` around lines 10 - 19, The preLoad wrapper only forwards dependencies, so callers cannot pass the same frameworkDeps set that main and StandaloneApp.preLoad use during boot. Update preLoad to accept frameworkDeps alongside dependencies, and pass both through to StandaloneApp.preLoad so the same module set can be warmed consistently.
🧹 Nitpick comments (5)
tegg/plugin/config/src/lib/ModuleScanner.ts (1)
25-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging swallowed errors for diagnosability.
The catch-all silently discards any error from
getFrameworkPath(not just "no framework resolvable" cases, e.g. malformedpackage.json). Adebug(...)call here would help diagnose unexpected misconfiguration without changing behavior.♻️ Suggested addition
private resolveFrameworkDir(): string | undefined { try { return getFrameworkPath({ baseDir: this.baseDir }); - } catch { + } catch (err) { + debug('resolveFrameworkDir failed: %o', err); // No package.json or no resolvable framework next to the app (e.g. // bare unit fixtures without node_modules) — app modules only. return undefined; } }🤖 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 25 - 33, The resolveFrameworkDir() method in ModuleScanner swallows all errors from getFrameworkPath(), so add a debug(...) log inside the catch before returning undefined. Use the same baseDir context from this.baseDir and keep the fallback behavior unchanged, but make sure unexpected failures (not just missing framework cases) are observable for diagnosis.tegg/core/metadata/src/impl/EggInnerObjectPrototypeImpl.ts (1)
99-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared prototype-construction path
EggInnerObjectPrototypeImpl.create()repeats the same field assembly andInjectObjectPrototypeFinder/IdenticalUtil.createProtoIdflow asEggPrototypeBuilder.create()/build(). A shared helper would reduce the chance of the two implementations drifting apart.🤖 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/core/metadata/src/impl/EggInnerObjectPrototypeImpl.ts` around lines 99 - 141, EggInnerObjectPrototypeImpl.create() duplicates the prototype assembly logic already present in EggPrototypeBuilder.create()/build(), so refactor the shared construction path into a common helper and have both call it. Keep the existing behavior for filepath, qualifiers, inject type/objects, InjectObjectPrototypeFinder.findInjectObjectPrototypes, and IdenticalUtil.createProtoId, but centralize the field mapping so the two implementations cannot drift apart.tegg/core/types/src/core-decorator/EggLifecycleProto.ts (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLiteral union collapses to
string, losing autocomplete for the five known lifecycle types.TypeScript widens
'LoadUnit' | ... | stringto juststring, so IDEs won't suggest the five official lifecycle type names when authors declare customEggLifecycleProto({ type: ... })calls.♻️ Proposed fix using the `string & {}` trick to retain literal autocomplete
export interface CommonEggLifecycleProtoParams extends InnerObjectProtoParams { - type: 'LoadUnit' | 'LoadUnitInstance' | 'EggObject' | 'EggPrototype' | 'EggContext' | string; + type: 'LoadUnit' | 'LoadUnitInstance' | 'EggObject' | 'EggPrototype' | 'EggContext' | (string & {}); }Please confirm this project's TypeScript version supports this pattern as expected (it's a long-standing, TS-team-acknowledged trick, not an official feature).
🤖 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/core/types/src/core-decorator/EggLifecycleProto.ts` around lines 3 - 5, The CommonEggLifecycleProtoParams.type union in EggLifecycleProto is collapsing to plain string, which removes autocomplete for the known lifecycle names. Update the type definition to preserve the five literal options while still allowing custom strings, using the standard string & {}-style workaround in CommonEggLifecycleProtoParams. Keep the change localized to the type declaration so EggLifecycleProto and related EggLifecycleProto({ type: ... }) call sites retain IDE suggestions without changing runtime behavior.tegg/core/runtime/src/impl/ProvidedInnerObjectProto.ts (1)
78-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
getMetaDatalooks up metadata on a plain factory closure, which never carries decorator metadata.
this.objFactoryis an arrow function created ad hoc by the builder (() => innerObject.obj), never a decorated class, soMetadataUtil.getMetaDatahere will effectively always resolve toundefined. If any consumer ofEggPrototype.getMetaData(e.g. AOP/crosscut logic) relies on this for provided inner objects, it will silently no-op instead of surfacing a clear "not supported" signal.🤖 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/core/runtime/src/impl/ProvidedInnerObjectProto.ts` around lines 78 - 80, getMetaData on ProvidedInnerObjectProto is reading decorator metadata from objFactory, but objFactory is just the builder’s plain arrow closure and will never have metadata. Update ProvidedInnerObjectProto.getMetaData to stop delegating to MetadataUtil.getMetaData on objFactory, and instead either return a clear unsupported result or throw a descriptive error for provided inner objects so EggPrototype consumers like AOP/crosscut logic do not silently no-op.tegg/plugin/tegg/test/ModulePlugin.test.ts (1)
9-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider adding multi-app regression coverage for this lifecycle change.
This exercises the new module-plugin boot order in
ModuleHandler(inner-object load unit instantiated before business load units) with a single app instance. As per coding guidelines, "When changing loader, runtime, lifecycle, or eventbus behavior, add or update multi-app regression coverage (for example tests likeMultiApp.test.ts) to verify two concurrent apps do not cross-talk." GivenModuleHandler.init()/destroy()ordering was substantially reworked, a companion test booting two concurrentmodule-plugin-app-style apps would guard against state leaking acrossTeggScopebags.🤖 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/tegg/test/ModulePlugin.test.ts` around lines 9 - 51, Add multi-app regression coverage for the ModuleHandler lifecycle change by creating a test that boots two concurrent module-plugin-app instances and verifies their TeggScope state does not cross-talk. Use the existing ModulePlugin.test.ts setup and the ModuleHandler init/destroy behavior as reference points, and assert both apps independently load HelloService, initialize inner-object load units before business load units, and keep their createdLoadUnits and innerRegistry state isolated.Source: Coding guidelines
🤖 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/tegg/src/lib/ModuleConfigLoader.ts`:
- Around line 59-71: The module resolution tolerance logic is duplicated in
ModuleConfigLoader and is already drifting from the similar code in the config
plugin and StandaloneApp. Extract the shared “resolve path, check fs.existsSync,
then fall back to reference.name and skip config loading” flow into a common
helper on ModuleConfigUtil (or equivalent) and update ModuleConfigLoader to call
it, so the modulePath/moduleDirExists/moduleName/defaultConfig handling stays
consistent across all callers.
In `@tegg/plugin/tegg/src/lib/ModuleHandler.ts`:
- Around line 116-132: The teardown logic in ModuleHandler currently stops on
the first thrown error in the load unit destruction loops, which can skip later
cleanup including the final `#innerObjectLoadUnit` teardown. Update the destroy
path around the loadUnitInstances, loadUnits, and `#innerObjectLoadUnit` blocks to
isolate each destroyLoadUnitInstance/destroyLoadUnit call so one failure does
not abort the remaining cleanup. Collect any errors during the loops in
ModuleHandler and rethrow an aggregate after all teardown steps have been
attempted.
- Around line 84-107: The init flow in ModuleHandler.init only assigns
loadUnitInstances after all later steps succeed, which leaves the inner-object
instance untracked if a boot error happens after
instantiateInnerObjectLoadUnit(). Update ModuleHandler.init so the
innerObjectInstance is stored in this.loadUnitInstances immediately after it is
created, then append each LoadUnitInstance created in the loop; keep the later
CompatibleUtil and loadUnit handling unchanged.
In `@tegg/standalone/standalone/src/main.ts`:
- Around line 40-47: The success-path cleanup in main/appMain is inconsistent
because app.destroy() is not awaited before returning, which can let
StandaloneApp resolve before TeggScope teardown finishes; change the finally
block to await the destroy promise just like the init-failure path so scope
cleanup completes before exit. Also update the .catch in that same app.destroy()
call to follow the existing Error-guard pattern used in this file, so non-Error
rejections are handled safely without mutating e.message directly. Use the
app.destroy() cleanup logic and the appMain/main flow as the key locations, and
add or update a regression test covering sequential StandaloneApp/MultiApp-style
teardown ordering to confirm no scope cross-talk.
In `@tegg/standalone/standalone/src/StandaloneApp.ts`:
- Around line 160-164: The per-app config selection in StandaloneApp should not
mutate the process-global ModuleConfigUtil.configNames, since that breaks
isolation between concurrent StandaloneApp instances. Update `#initRuntime`,
doDestroy, and the ModuleConfigUtil.readModuleNameSync/loadModuleConfigSync path
so configNames is passed or stored via TeggScope-backed instance state instead
of a shared static, keeping each app’s env-specific module.default/module.<env>
lookup independent.
In `@wiki/concepts/tegg-module-plugin.md`:
- Around line 5-13: The source_files list is missing the actual host-entrypoint
sources that back the egg-host feeding claim. Update the wiki page’s
source_files in the tegg-module-plugin section to include the plugin boot
entrypoints that register the hard-fed inner-object lists, such as the app.ts
entrypoints in the AOP, config, and DAL plugins, alongside the existing runtime
and handler symbols. Keep the listed sources aligned with the claim’s
traceability requirements so the major code paths are represented.
---
Outside diff comments:
In `@tegg/core/loader/src/LoaderFactory.ts`:
- Around line 61-62: Each ModuleDescriptor is currently sharing the same
multiInstanceClazzList, so later iterations can mutate earlier descriptors and
confuse GlobalGraph.create() attribution. Move the multiInstanceClazzList
initialization so it is created per module inside the LoaderFactory logic that
builds each ModuleDescriptor, and make sure each descriptor receives its own
fresh array before pushing multi-instance classes.
In `@tegg/standalone/standalone/src/main.ts`:
- Around line 10-19: The preLoad wrapper only forwards dependencies, so callers
cannot pass the same frameworkDeps set that main and StandaloneApp.preLoad use
during boot. Update preLoad to accept frameworkDeps alongside dependencies, and
pass both through to StandaloneApp.preLoad so the same module set can be warmed
consistently.
---
Nitpick comments:
In `@tegg/core/metadata/src/impl/EggInnerObjectPrototypeImpl.ts`:
- Around line 99-141: EggInnerObjectPrototypeImpl.create() duplicates the
prototype assembly logic already present in
EggPrototypeBuilder.create()/build(), so refactor the shared construction path
into a common helper and have both call it. Keep the existing behavior for
filepath, qualifiers, inject type/objects,
InjectObjectPrototypeFinder.findInjectObjectPrototypes, and
IdenticalUtil.createProtoId, but centralize the field mapping so the two
implementations cannot drift apart.
In `@tegg/core/runtime/src/impl/ProvidedInnerObjectProto.ts`:
- Around line 78-80: getMetaData on ProvidedInnerObjectProto is reading
decorator metadata from objFactory, but objFactory is just the builder’s plain
arrow closure and will never have metadata. Update
ProvidedInnerObjectProto.getMetaData to stop delegating to
MetadataUtil.getMetaData on objFactory, and instead either return a clear
unsupported result or throw a descriptive error for provided inner objects so
EggPrototype consumers like AOP/crosscut logic do not silently no-op.
In `@tegg/core/types/src/core-decorator/EggLifecycleProto.ts`:
- Around line 3-5: The CommonEggLifecycleProtoParams.type union in
EggLifecycleProto is collapsing to plain string, which removes autocomplete for
the known lifecycle names. Update the type definition to preserve the five
literal options while still allowing custom strings, using the standard string &
{}-style workaround in CommonEggLifecycleProtoParams. Keep the change localized
to the type declaration so EggLifecycleProto and related EggLifecycleProto({
type: ... }) call sites retain IDE suggestions without changing runtime
behavior.
In `@tegg/plugin/config/src/lib/ModuleScanner.ts`:
- Around line 25-33: The resolveFrameworkDir() method in ModuleScanner swallows
all errors from getFrameworkPath(), so add a debug(...) log inside the catch
before returning undefined. Use the same baseDir context from this.baseDir and
keep the fallback behavior unchanged, but make sure unexpected failures (not
just missing framework cases) are observable for diagnosis.
In `@tegg/plugin/tegg/test/ModulePlugin.test.ts`:
- Around line 9-51: Add multi-app regression coverage for the ModuleHandler
lifecycle change by creating a test that boots two concurrent module-plugin-app
instances and verifies their TeggScope state does not cross-talk. Use the
existing ModulePlugin.test.ts setup and the ModuleHandler init/destroy behavior
as reference points, and assert both apps independently load HelloService,
initialize inner-object load units before business load units, and keep their
createdLoadUnits and innerRegistry state isolated.
🪄 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: b5819719-d3f0-4e63-9f9f-28a26cccc964
⛔ Files ignored due to path filters (8)
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__/dal.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!**/*.snap
📒 Files selected for processing (119)
tegg/core/aop-decorator/src/CrosscutAdviceFactory.tstegg/core/aop-runtime/package.jsontegg/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/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/inner-object-decorators.test.tstegg/core/loader/src/LoaderFactory.tstegg/core/loader/src/LoaderUtil.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/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/core-decorator/EggLifecycleProto.tstegg/core/types/src/core-decorator/InnerObjectProto.tstegg/core/types/src/core-decorator/Prototype.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/test/aop.test.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/ReadModule.test.tstegg/plugin/dal/package.jsontegg/plugin/dal/src/app.tstegg/plugin/dal/src/index.tstegg/plugin/dal/src/lib/DalModuleLoadUnitHook.tstegg/plugin/dal/src/lib/DalTableEggPrototypeHook.tstegg/plugin/dal/src/lib/MysqlDataSourceManagerObject.tstegg/plugin/dal/src/lib/TransactionPrototypeHook.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/ModulePlugin.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/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.tswiki/concepts/tegg-module-plugin.mdwiki/index.mdwiki/log.md
💤 Files with no reviewable changes (6)
- tegg/standalone/standalone/src/ConfigSourceLoadUnitHook.ts
- tegg/standalone/standalone/src/StandaloneInnerObject.ts
- tegg/standalone/standalone/src/StandaloneLoadUnit.ts
- tegg/standalone/standalone/src/Runner.ts
- tegg/plugin/tegg/package.json
- tegg/standalone/standalone/src/StandaloneInnerObjectProto.ts
| // load module.yml and module.env.yml by default | ||
| // Always set configNames for this app invocation, since destroy() clears it | ||
| // asynchronously and may not have completed before the next app is created. | ||
| ModuleConfigUtil.configNames = opts.env ? ['module.default', `module.${opts.env}`] : ['module.default']; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Mutating the process-global ModuleConfigUtil.configNames breaks the per-app isolation this file otherwise guarantees via TeggScope.
Every other piece of per-app mutable state in this class (#moduleConfigs, #runtimeConfig, #moduleReferences, loadUnits, scopeBag, ...) is instance-scoped, and the class comments explicitly promise "multiple StandaloneApps in one process stay isolated." ModuleConfigUtil.configNames, however, is set as a static/global value in #initRuntime and cleared globally in doDestroy via ModuleConfigUtil.setConfigNames(undefined). The comment on Line 162 itself concedes the race: "since destroy() clears it asynchronously and may not have completed before the next app is created." If two StandaloneApp instances are alive concurrently (e.g. two appMain() calls in flight, or app B booting while app A is still tearing down), one app's env-specific config file names (module.<env>) can silently leak into or get cleared out from under the other, causing the wrong module config to load — undermining the multi-app isolation this rewrite is meant to provide.
As per coding guidelines, "In Tegg source code, do not introduce new process-global mutable runtime state (for example static fields, Maps, or singletons that hold per-app data); use TeggScope-backed slots for per-app state instead." Consider threading configNames explicitly through ModuleConfigUtil.readModuleNameSync/loadModuleConfigSync calls (or backing it with a TeggScope slot resolved via this.scopeBag) instead of a shared static field.
Also applies to: 389-389
🤖 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/src/StandaloneApp.ts` around lines 160 - 164, The
per-app config selection in StandaloneApp should not mutate the process-global
ModuleConfigUtil.configNames, since that breaks isolation between concurrent
StandaloneApp instances. Update `#initRuntime`, doDestroy, and the
ModuleConfigUtil.readModuleNameSync/loadModuleConfigSync path so configNames is
passed or stored via TeggScope-backed instance state instead of a shared static,
keeping each app’s env-specific module.default/module.<env> lookup independent.
Source: Coding guidelines
There was a problem hiding this comment.
这里不改。当前 ModuleConfigUtil.configNames 已经不是 process-global mutable state:getter/setter 内部走 TeggScope slot(CONFIG_NAMES_SLOT)。Standalone 的 init() / destroy() 都包在 this.runInScope(this.scopeBag, ...) 里,所以 set/clear configNames 都落在当前 app scope,不会跨 StandaloneApp 泄漏。这里保留 ModuleConfigUtil.configNames 这个 API 入口,是为了复用既有 module config 读取路径,不再额外引入一套参数穿透。
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f "ModuleConfig.ts" tegg/core/common-util/src
echo "---"
rg -n "configNames|CONFIG_NAMES_SLOT|setConfigNames" tegg/core/common-util/src/ModuleConfig.ts -A3 -B3Length of output: 2980
Confirmed — configNames is backed by a TeggScope slot, not process-global state, so the isolation concern doesn't apply as originally described.
@gxkl 确认了,ModuleConfigUtil.configNames 的 getter/setter 确实通过 TeggScope slot(CONFIG_NAMES_SLOT)存取,而不是进程级全局静态字段,init()/destroy() 也都在各自 scopeBag 内执行,所以不存在跨 StandaloneApp 实例互相污染的问题。撤回这条意见,感谢澄清。
| this.eggObjectAopHook = new EggObjectAopHook(); | ||
| } | ||
|
|
||
| configDidLoad(): void { |
There was a problem hiding this comment.
已改:空 hook 方法直接删掉,不再保留空实现。后续把 AOP context hook 也改成 inner object 后,app boot 里不再需要这类手动空占位。
| this.app.eggPrototypeLifecycleUtil.deleteLifecycle(this.eggPrototypeCrossCutHook); | ||
| this.app.loadUnitLifecycleUtil.deleteLifecycle(this.loadUnitAopHook); | ||
| this.app.eggObjectLifecycleUtil.deleteLifecycle(this.eggObjectAopHook); | ||
| this.app.eggContextLifecycleUtil.deleteLifecycle(this.aopContextHook); |
There was a problem hiding this comment.
已改:这里原来是手动注册/删除 AopContextHook,现在改成 AOP 模块自己的 inner object/lifecycle proto,由 inner object load unit 管理生命周期,不再需要 app boot 里手动挂载这行。
| @InnerObjectProto({ name: 'mysqlDataSourceManager', accessLevel: AccessLevel.PUBLIC }) | ||
| export class MysqlDataSourceManagerObject { | ||
| constructor() { | ||
| return MysqlDataSourceManager.instance; |
There was a problem hiding this comment.
这个显得很奇怪了,应该可以改成直接继承而不是返回一个 instance?
There was a problem hiding this comment.
已改,而且不再只处理 MysqlDataSourceManager 这个特例:DAL 三个 manager 都改成由 dal 模块自己声明 inner object(MysqlDataSourceManager / SqlMapManager / TableModelManager),消费侧通过 DI 获取;同时移除了原来的 constructor return instance wrapper 和 app.mysqlDataSourceManager 兼容扩展,避免静态单例与 inner object 混用。
| // every object they may hook has been destroyed. | ||
| if (this.loadUnitInstances) { | ||
| for (const instance of this.loadUnitInstances) { | ||
| for (const instance of [...this.loadUnitInstances].reverse()) { |
There was a problem hiding this comment.
更新:这块后续 review 又指出,不能只保证 InnerObjectLoadUnit 最后销毁,还必须保证 inner-object instance 不早于 business LoadUnitFactory.destroyLoadUnit() 销毁,否则 lifecycle proto 会提前注销。已在 commit f347717 精确调整为:先反向销毁 business load unit instances,再反向销毁 business load units,最后销毁 inner-object instance 和 inner-object load unit。这样 AOP/DAL/config 等 lifecycle protos 在业务实例销毁和业务 load unit metadata 销毁期间都仍然可用;同时保留逐个 safe destroy + AggregateError 汇总,避免前一个销毁失败导致后续 load unit 不清理。
Code Review: 声明式 module plugin 机制整体架构清晰,happy path 测试充分。但本次评审最核心的主题是:hook 的投递方式从"无条件命令式注册"变成了"扫描 + 提升(promote)流水线",而这条流水线的每一种失败模式都是静默跳过 —— 若干现实部署形态会在没有任何报错的情况下丢失 发现(按严重程度排序)1.
|
79594a6 to
facb7f1
Compare
| reference: { | ||
| name: moduleName, | ||
| name: resolved.name, | ||
| package: reference.package, | ||
| path: reference.path, | ||
| }, |
There was a problem hiding this comment.
已改,方案是只修正派生出来的 moduleConfigs[resolved.name].reference.path:这里现在存 resolved.path,与 @eggjs/tegg-config 侧保持一致,避免 relative path 泄漏进 moduleConfigs。注意这不反写 app.moduleReferences 本体,manifest/module reference 的原始语义仍保持不变。
There was a problem hiding this comment.
Actionable comments posted: 4
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)
48-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLabel the example fence as
ts.This is a TypeScript example, so the bare fence trips markdownlint and is less explicit for readers.
Fix
-``` +```ts🤖 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 48 - 58, The README example fence is unlabeled, so update the fenced code block in the standalone example to use the TypeScript language tag. Locate the sample around the await main call in the README and change the bare triple-backtick fence to a ts-labeled fence so markdownlint passes and the snippet is clearer for readers.Source: Linters/SAST tools
🧹 Nitpick comments (3)
tegg/core/core-decorator/src/decorator/DefineModuleQualifier.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
.jsextension in import specifier.As per coding guidelines, Tegg code must use ESM-only imports and
.jsextensions in import specifiers. The import../util/index.tsuses a.tsextension instead of.js.♻️ Proposed fix
-import { QualifierUtil } from '../util/index.ts'; +import { QualifierUtil } from '../util/index.js';🤖 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/core/core-decorator/src/decorator/DefineModuleQualifier.ts` at line 4, The import in DefineModuleQualifier should use the ESM .js specifier instead of a .ts extension. Update the QualifierUtil import in DefineModuleQualifier.ts to point at the generated .js path in the same relative location, following the repo’s ESM-only import convention.Source: Coding guidelines
tegg/core/types/src/core-decorator/EggLifecycleProto.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
.jsextension in import specifiers.As per coding guidelines, Tegg code must use ESM-only imports and
.jsextensions in import specifiers. Both imports use.tsextensions.♻️ Proposed fix
-import type { InnerObjectProtoParams } from './InnerObjectProto.ts'; -import type { EggLifecycleType } from './model/EggLifecycleInfo.ts'; +import type { InnerObjectProtoParams } from './InnerObjectProto.js'; +import type { EggLifecycleType } from './model/EggLifecycleInfo.js';🤖 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/core/types/src/core-decorator/EggLifecycleProto.ts` around lines 1 - 2, The imports in EggLifecycleProto should follow the ESM guideline by using .js specifiers instead of .ts. Update the type-only imports from InnerObjectProto and EggLifecycleInfo in EggLifecycleProto.ts to reference the corresponding .js paths so the module resolution stays consistent with the rest of Tegg.Source: Coding guidelines
tegg/plugin/config/src/lib/ModuleScanner.ts (1)
48-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional:
resolveParentFrameworkDirreadsframeworkDir/package.jsontwice.The method explicitly reads and parses
frameworkDir/package.jsonto checkpkg.egg.framework, then callsthis.resolveFrameworkDir(frameworkDir)which internally re-reads the same file viagetFrameworkPath. The explicit check is necessary to avoid defaulting to'egg'when no parent is declared, but the double-read is avoidable ifgetFrameworkPathaccepts a pre-resolved framework name.♻️ Optional: avoid double-read if getFrameworkPath supports explicit framework
private resolveParentFrameworkDir(frameworkDir: string): string | undefined { - let pkg: { egg?: { framework?: unknown } }; - try { - pkg = JSON.parse(fs.readFileSync(path.join(frameworkDir, 'package.json'), 'utf8')); - } catch (err) { - debug( - 'read framework package failed, frameworkDir: %s, err: %s', - frameworkDir, - err instanceof Error ? err.message : String(err), - ); - return undefined; - } - const framework = pkg.egg?.framework; - if (typeof framework !== 'string' || !framework) { - return undefined; - } - return this.resolveFrameworkDir(frameworkDir); + // If getFrameworkPath supports an explicit framework option, + // pass the resolved name directly to avoid re-reading package.json: + // return getFrameworkPath({ baseDir: frameworkDir, framework }); + // Otherwise, the current approach is correct — keep as-is. + return this.resolveFrameworkDir(frameworkDir); }🤖 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 48 - 65, `resolveParentFrameworkDir` in `ModuleScanner` is reading `frameworkDir/package.json` twice: once to inspect `pkg.egg.framework`, then again indirectly through `resolveFrameworkDir`/`getFrameworkPath`. Keep the explicit parent-framework check, but refactor so the parsed framework value is passed through to the framework resolution path instead of re-reading the same package file, or otherwise update `getFrameworkPath`/`resolveFrameworkDir` to accept an explicit framework name.
🤖 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/core/aop-runtime/src/AopContextAdviceRegistry.ts`:
- Line 10: `AopContextAdviceRegistry` is currently marked with
`@InnerObjectProto()`, which makes it private and prevents `AopContextHook` from
injecting it across package boundaries. Update the declaration in
`AopContextAdviceRegistry` so the registry is public/exported in the proto
metadata, keeping the existing class name and related injection points intact.
In `@tegg/core/aop-runtime/src/LoadUnitAopHook.ts`:
- Around line 19-20: The injected aopContextAdviceRegistry field in
LoadUnitAopHook should not create its own AopContextAdviceRegistry fallback;
remove the default initializer from the `@Inject`()-decorated property so DI
resolution is the single source of truth. Keep the field injection in
LoadUnitAopHook aligned with AopContextHook by relying on container injection
only, which will make both hooks fail fast on missing dependencies instead of
silently diverging.
In `@tegg/plugin/config/test/ModuleScanner.test.ts`:
- Around line 5-6: The imports in ModuleScanner.test.ts use TypeScript
extensions, which violates the ESM-only import guideline for tegg test files.
Update the ModuleScanner and getFixtures import specifiers to use .js
extensions, matching the pattern used in other tests such as
ModuleConfig.test.ts, and keep the relative paths otherwise unchanged.
In `@wiki/concepts/tegg-module-plugin.md`:
- Around line 5-16: The wiki page’s source_files list is missing the egg-host
loader referenced in the boot-order discussion. Update the list in
tegg-module-plugin docs to include EggModuleLoader so the documented
initGraph()/load() flow is traceable; keep the existing source_files block
aligned with the other plugin/runtime symbols already listed.
---
Outside diff comments:
In `@tegg/standalone/standalone/README.md`:
- Around line 48-58: The README example fence is unlabeled, so update the fenced
code block in the standalone example to use the TypeScript language tag. Locate
the sample around the await main call in the README and change the bare
triple-backtick fence to a ts-labeled fence so markdownlint passes and the
snippet is clearer for readers.
---
Nitpick comments:
In `@tegg/core/core-decorator/src/decorator/DefineModuleQualifier.ts`:
- Line 4: The import in DefineModuleQualifier should use the ESM .js specifier
instead of a .ts extension. Update the QualifierUtil import in
DefineModuleQualifier.ts to point at the generated .js path in the same relative
location, following the repo’s ESM-only import convention.
In `@tegg/core/types/src/core-decorator/EggLifecycleProto.ts`:
- Around line 1-2: The imports in EggLifecycleProto should follow the ESM
guideline by using .js specifiers instead of .ts. Update the type-only imports
from InnerObjectProto and EggLifecycleInfo in EggLifecycleProto.ts to reference
the corresponding .js paths so the module resolution stays consistent with the
rest of Tegg.
In `@tegg/plugin/config/src/lib/ModuleScanner.ts`:
- Around line 48-65: `resolveParentFrameworkDir` in `ModuleScanner` is reading
`frameworkDir/package.json` twice: once to inspect `pkg.egg.framework`, then
again indirectly through `resolveFrameworkDir`/`getFrameworkPath`. Keep the
explicit parent-framework check, but refactor so the parsed framework value is
passed through to the framework resolution path instead of re-reading the same
package file, or otherwise update `getFrameworkPath`/`resolveFrameworkDir` to
accept an explicit framework name.
🪄 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: 2e20b350-6955-41a9-bcbf-d5591ccb540a
⛔ 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/plugin/dal/src/app/extend/application.ts
- tegg/plugin/dal/package.json
- tegg/standalone/standalone/src/StandaloneInnerObject.ts
- tegg/standalone/standalone/src/StandaloneInnerObjectProto.ts
- tegg/standalone/standalone/src/Runner.ts
- tegg/standalone/standalone/src/ConfigSourceLoadUnitHook.ts
- tegg/plugin/tegg/package.json
- tegg/standalone/standalone/src/StandaloneLoadUnit.ts
- tegg/plugin/dal/src/app.ts
✅ Files skipped from review due to trivial changes (31)
- tegg/plugin/config/test/fixtures/framework-chain/app/package.json
- tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/package.json
- tegg/core/types/src/core-decorator/model/index.ts
- tegg/core/metadata/src/model/graph/index.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/FetchRouter.ts
- tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/FooEggContextHook.ts
- wiki/index.md
- tegg/standalone/standalone/test/fixtures/dal-manager-inject/package.json
- tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/package.json
- tegg/plugin/config/test/fixtures/framework-same-path/app/package.json
- tegg/plugin/config/test/fixtures/framework-cycle/app/package.json
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/package.json
- tegg/plugin/config/test/fixtures/framework-module-json/app/package.json
- tegg/core/metadata/src/impl/EggPrototypeImpl.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/module.json
- tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/package.json
- tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/package.json
- tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/package.json
- tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/innerBar.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/package.json
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Foo.ts
- tegg/plugin/config/src/lib/ConfigSourceLoadUnitHook.ts
- tegg/standalone/standalone/tsdown.config.ts
- tegg/core/metadata/src/impl/index.ts
- tegg/standalone/standalone/test/fixtures/logger-option/package.json
- tegg/core/types/src/core-decorator/enum/Qualifier.ts
- tegg/core/metadata/test/ModuleDescriptorDumper.test.ts
- tegg/plugin/aop/src/InnerObjects.ts
- wiki/log.md
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/package.json
- tegg/plugin/tegg/test/BundledAppBoot.test.ts
🚧 Files skipped from review as they are similar to previous changes (68)
- tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/FooEggPrototypeHook.ts
- tegg/core/types/src/core-decorator/index.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/ControllerHook.ts
- tegg/core/aop-runtime/src/AopGraphHookRegistrar.ts
- tegg/standalone/standalone/test/fixtures/egg-context-lifecycle-proto/Foo.ts
- tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/FooLoadUnitInstanceHook.ts
- tegg/core/aop-runtime/src/EggObjectAopHook.ts
- tegg/core/types/src/core-decorator/Prototype.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/config.default.ts
- tegg/core/aop-runtime/src/index.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/Runner.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/package.json
- tegg/standalone/standalone/test/fixtures/load-unit-instance-lifecycle-proto/Foo.ts
- tegg/standalone/standalone/test/fixtures/inner-object-proto/foo.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/HelloService.ts
- tegg/standalone/standalone/test/fixtures/logger-option/foo.ts
- tegg/standalone/standalone/test/fixtures/invalid-inner-object-inject/foo.ts
- tegg/core/types/src/core-decorator/InnerObjectProto.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/InnerRegistry.ts
- tegg/core/loader/test/fixtures/modules/module-with-inner-object/HelloService.ts
- tegg/core/aop-decorator/src/CrosscutAdviceFactory.ts
- tegg/core/types/src/metadata/model/ProtoDescriptor.ts
- tegg/plugin/aop/package.json
- tegg/standalone/standalone/src/index.ts
- tegg/core/core-decorator/src/decorator/index.ts
- tegg/core/core-decorator/src/decorator/InnerObjectProto.ts
- tegg/standalone/standalone/test/ModulePlugin.test.ts
- tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/FooEggObjectHook.ts
- tegg/core/core-decorator/src/decorator/EggLifecycleProto.ts
- tegg/standalone/standalone/test/fixtures/inner-object-proto/package.json
- tegg/plugin/aop/test/aop.test.ts
- tegg/plugin/tegg/test/ModulePlugin.test.ts
- tegg/core/loader/test/LoaderInnerObject.test.ts
- tegg/core/metadata/src/model/ProtoDescriptorHelper.ts
- tegg/core/metadata/src/model/graph/GlobalGraph.ts
- tegg/plugin/config/package.json
- tegg/core/aop-runtime/test/aop-runtime.test.ts
- tegg/core/aop-runtime/package.json
- tegg/core/metadata/src/impl/InjectObjectPrototypeFinder.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/modules/plugin-module/PluginHooks.ts
- tegg/standalone/standalone/test/fixtures/inner-object-proto/innerBar.ts
- tegg/plugin/config/test/ReadModule.test.ts
- tegg/standalone/standalone/test/fixtures/load-unit-lifecycle-proto/FooLoadUnitHook.ts
- tegg/plugin/tegg/test/fixtures/apps/module-plugin-app/config/plugin.ts
- tegg/core/runtime/src/impl/ProvidedInnerObjectProto.ts
- tegg/standalone/standalone/test/fixtures/egg-object-lifecycle-proto/Foo.ts
- tegg/standalone/standalone/test/fixtures/egg-prototype-lifecycle-proto/Foo.ts
- tegg/plugin/dal/src/lib/DalModuleLoadUnitHook.ts
- tegg/core/types/package.json
- tegg/core/aop-runtime/src/EggPrototypeCrossCutHook.ts
- tegg/standalone/standalone/src/main.ts
- tegg/core/test-util/src/LoaderUtil.ts
- tegg/core/runtime/src/impl/index.ts
- tegg/core/core-decorator/src/util/PrototypeUtil.ts
- tegg/core/metadata/src/model/ModuleDescriptor.ts
- tegg/core/metadata/src/impl/EggPrototypeBuilder.ts
- tegg/core/runtime/src/impl/InnerObjectLoadUnit.ts
- tegg/core/runtime/src/impl/InnerObjectLoadUnitInstance.ts
- tegg/core/core-decorator/test/inner-object-decorators.test.ts
- tegg/plugin/tegg/src/app.ts
- tegg/standalone/standalone/package.json
- tegg/core/runtime/src/impl/InnerObjectLoadUnitBuilder.ts
- tegg/standalone/standalone/src/EggModuleLoader.ts
- tegg/plugin/tegg/src/lib/ModuleHandler.ts
- tegg/core/metadata/src/model/graph/ProtoGraphUtils.ts
- tegg/core/runtime/src/impl/EggInnerObjectImpl.ts
- tegg/standalone/standalone/src/StandaloneApp.ts
- tegg/standalone/standalone/test/index.test.ts
facb7f1 to
80b9d49
Compare
f347717 to
b774e07
Compare
| try { | ||
| await fs.writeFile(tmpPath, ModuleDescriptorDumper.stringifyDescriptor(desc)); | ||
| await fs.rename(tmpPath, dumpPath); | ||
| } finally { |
b774e07 to
5c42a6d
Compare
| import assert from 'node:assert/strict'; | ||
| import path from 'node:path'; | ||
|
|
||
| import { EggPrototypeNotFound } from '@eggjs/metadata'; | ||
| import { describe, it } from 'vitest'; | ||
|
|
||
| import { main } from '../src/index.ts'; | ||
|
|
||
| describe('standalone/standalone/test/ModulePlugin.test.ts', () => { | ||
| const getFixture = (name: string) => path.join(__dirname, 'fixtures', name); | ||
|
|
| const resolvedRef = { | ||
| path: resolved.path, | ||
| name: reference.name, | ||
| package: reference.package, | ||
| optional: reference.optional, | ||
| loaderType: reference.loaderType, | ||
| }; |
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
StandaloneApp, including manifest reuse andloaderFS, and updated standalone entrypoints.innerObjectHandlersnaming).