coderabbitai[bot]
commented
1 month ago Walkthrough
The changes enhance the dependency injection framework within the Egg.js ecosystem. Key updates include the addition of new decorators for managing injections, modifications to existing decorator functions to support constructor injection, and the introduction of new utility methods for handling metadata and constructor argument names. New classes and interfaces have been created to facilitate these features, along with corresponding test cases to ensure functionality. Additionally, several configuration files and modules have been added to support dynamic features and application structure.
Changes
| File(s) | Change Summary |
|---|---|
README.md |
Added an example for the HelloService class demonstrating the use of the Inject annotation in the constructor. |
core/common-util/src/ObjectUtils.ts |
Introduced getConstructorArgNameList method for retrieving constructor argument names from a class. |
core/common-util/test/ObjectUtil.test.ts |
Added decorators InitTypeQualifier, ModuleQualifier, and Inject, along with tests for getConstructorArgNameList. |
core/core-decorator/src/decorator/*.ts |
Updated several decorators (ConfigSourceQualifier, EggQualifier, InitTypeQualifier, Inject, ModuleQualifier) to accept an optional parameterIndex to enhance injection flexibility. |
core/core-decorator/src/util/*.ts |
Introduced new methods in MetadataUtil, PrototypeUtil, and QualifierUtil to manage injection types and metadata. |
core/metadata/src/impl/*.ts |
Modified EggPrototypeBuilder and EggPrototypeImpl to support constructor injection and manage injection types effectively. |
core/runtime/src/impl/*.ts |
Enhanced EggObjectImpl and EggObjectUtil to support constructor-based dependency injection and proxy creation for dynamic property manipulation. |
core/types/core-decorator/enum/*.ts |
Added InjectType enum and CONSTRUCTOR_QUALIFIER_META_DATA symbol for managing injection metadata. |
core/types/core-decorator/model/*.ts |
Introduced InjectConstructorInfo and InjectConstructorProto interfaces for managing constructor injection details. |
plugin/tegg/test/*.ts |
Added tests for module configuration and controller functionality in the Egg.js application. |
plugin/tegg/test/fixtures/apps/constructor-module-config/*.ts |
Introduced various classes, controllers, and configuration files to support dynamic module features and application structure. |
plugin/tegg/test/fixtures/apps/constructor-module-config/config/*.js |
Added configuration files for module settings and dynamic features. |
plugin/tegg/test/fixtures/apps/constructor-module-config/modules/module-with-config/*.ts |
Created new modules and classes with dependency injection capabilities. |
plugin/tegg/test/fixtures/apps/constructor-module-config/package.json |
Introduced package.json for the application and modules to define dependencies and configurations. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant App
participant Controller
participant Service
User->>App: Request /config
App->>Controller: Call baseDir()
Controller->>Service: Retrieve configuration
Service-->>Controller: Return config values
Controller-->>App: Send response with config
App-->>User: Return response🐰 In the code, we find new ways,
To inject and log our days.
With constructors now in play,
Dependencies dance and sway.
In Egg.js, we hop with glee,
Celebrating changes, oh so free! 🎉
📜 Recent review details
**Configuration used: CodeRabbit UI** **Review profile: CHILL**📥 Commits
Files that changed from the base of the PR and between 35e4a06349b4a583d0cfb61046001646d26290b4 and 16ee600c77f89f0b7761652fd64028469afef98e.📒 Files selected for processing (3)
* core/core-decorator/src/util/QualifierUtil.ts (2 hunks) * core/core-decorator/test/decorators.test.ts (3 hunks) * core/metadata/test/LoadUnit.test.ts (1 hunks)🚧 Files skipped from review as they are similar to previous changes (2)
* core/core-decorator/test/decorators.test.ts * core/metadata/test/LoadUnit.test.ts🔇 Additional comments (1)
core/core-decorator/src/util/QualifierUtil.ts (1)
`27-35`: _:hammer_and_wrench: Refactor suggestion_ _:warning: Potential issue_ **Address potential issues in `addInjectQualifier` method** There are a few potential issues in this method that need to be addressed: 1. Potential out-of-bounds access when using `parameterIndex`. 2. Possible `undefined` access when using `property!`. To resolve these issues, please consider the following changes: ```diff static addInjectQualifier(clazz: EggProtoImplClass, property: PropertyKey | undefined, parameterIndex: number | undefined, attribute: QualifierAttribute, value: QualifierValue) { if (typeof parameterIndex === 'number') { const argNames = ObjectUtils.getConstructorArgNameList(clazz); + if (parameterIndex >= 0 && parameterIndex < argNames.length) { const argName = argNames[parameterIndex]; QualifierUtil.addProperQualifier(clazz, argName, attribute, value); + } else { + throw new Error(`Invalid parameterIndex ${parameterIndex} for class ${clazz.name}`); + } } else { + if (property !== undefined) { QualifierUtil.addProperQualifier((clazz as any).constructor, property!, attribute, value); + } else { + throw new Error(`Property key is undefined for class ${clazz.name}`); + } } } ``` These changes will: 1. Add a bounds check for `parameterIndex` to prevent out-of-bounds access. 2. Add a check for `property` to ensure it's not `undefined` before using it. 3. Throw informative errors when invalid inputs are provided. Additionally, consider refactoring the method to improve readability and reduce duplication: ```typescript static addInjectQualifier(clazz: EggProtoImplClass, property: PropertyKey | undefined, parameterIndex: number | undefined, attribute: QualifierAttribute, value: QualifierValue) { let targetClass: EggProtoImplClass; let targetProperty: PropertyKey; if (typeof parameterIndex === 'number') { const argNames = ObjectUtils.getConstructorArgNameList(clazz); if (parameterIndex < 0 || parameterIndex >= argNames.length) { throw new Error(`Invalid parameterIndex ${parameterIndex} for class ${clazz.name}`); } targetClass = clazz; targetProperty = argNames[parameterIndex]; } else if (property !== undefined) { targetClass = (clazz as any).constructor; targetProperty = property; } else { throw new Error(`Either parameterIndex or property must be provided for class ${clazz.name}`); } QualifierUtil.addProperQualifier(targetClass, targetProperty, attribute, value); } ``` This refactored version: 1. Centralizes the error handling. 2. Reduces duplication by using a single call to `addProperQualifier`. 3. Improves readability by clearly separating the logic for determining the target class and property. Note: This review comment addresses the issues raised in the past review comments, which are still valid for this code segment.
Checklist
npm testpassesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
New Features
Injectannotation within theHelloServiceclass.ConstructorObjectclass created to showcase dependency injection with various decorators.MetadataUtilclass.Bug Fixes
Tests
Documentation