Adding Dependency Injection system #15
Conversation
ben-ryder
commented
Jun 2, 2022
•
ben-ryder
commented
- Adding DI system
- Add system for controllers and their dependencies
- Add system for middleware functions
…DependencyContainer
| @@ -32,6 +32,7 @@ module.exports = { | |||
| "semi": [ | |||
| "error", | |||
| "always" | |||
| ] | |||
| ], | |||
| "@typescript-eslint/no-explicit-any": "off" | |||
There was a problem hiding this comment.
Remove in favor of file level disabling
| verbose: true, | ||
| silent: false, | ||
| transform: { | ||
| "^.+\\.tsx?$": "ts-jest", |
There was a problem hiding this comment.
Is this needed, there's no .tsx files anywhere
| @@ -0,0 +1 @@ | |||
| import "reflect-metadata"; | |||
There was a problem hiding this comment.
Add comment explaining why this is needed for jest
| @@ -14,5 +14,10 @@ export function Controller(routePrefix = ""): ClassDecorator { | |||
| if (!Reflect.hasMetadata(MetadataKeys.ROUTES, target)) { | |||
| Reflect.defineMetadata(MetadataKeys.ROUTES, [], target); | |||
| } | |||
|
|
|||
| // Automatically setup dependency metadata here so users don't need to add @Injectable to controllers | |||
| const dependencyKey = Symbol.for(target.toString()); | |||
There was a problem hiding this comment.
Add ability for users to set custom identifier via @Controller like you can do with @Injectable
| // Automatically setup dependency metadata here so users don't need to add @Injectable to controllers | ||
| const dependencyKey = Symbol.for(target.toString()); | ||
| Reflect.defineMetadata(MetadataKeys.DEPENDENCY_KEY, dependencyKey, target); | ||
| Reflect.defineMetadata(MetadataKeys.DEPENDENCY_CONFIG, {mode: "global"}, target); |
There was a problem hiding this comment.
Does this need to add metadata to target.prototype instead?
| * @param options - options for customising how KangoJS works. | ||
| */ | ||
| constructor(app: Application, options: KangoJSOptions) { | ||
| this.app = app; | ||
| this.router = Router(); |
There was a problem hiding this comment.
Try to add via DI system?
packages/core/src/kangojs.ts
Outdated
| this.router = Router(); | ||
|
|
||
| // Setup dependency container | ||
| // no dependencies are added till .bootstrap to allow for overwrites of the Express router/app etc |
There was a problem hiding this comment.
update comment if this isn't the case
packages/core/src/kangojs.ts
Outdated
| * @param app - An Express application | ||
| * @param options - options for customising how KangoJS works. | ||
| */ | ||
| constructor(app: Application, options: KangoJSOptions) { |
There was a problem hiding this comment.
Should Express app creation be moved to within the framework?
This might make it easier in the future to add middleware level DI
| } | ||
|
|
||
|
|
||
| export abstract class LoggerBase { |
| identifier: "test-global" | ||
| }) | ||
| // @ts-ignore | ||
| class TestSingletonDependency { |
There was a problem hiding this comment.
Try and find way to make this quiet without ts-ignore.
Issue is that IDE is complaining that decorators are experimental even though I've got experimentalDecorators enabled in tsconfig
…st deps for template
…quired by v2 kangojs/core
…ore and adding middleware DI
… with default log level
|
closing as old PR, feature already in v2 alpha |
