[feat] Add support for cleaning up disposable instances#132
[feat] Add support for cleaning up disposable instances#132Xapphire13 wants to merge 1 commit intomasterfrom
Conversation
|
|
||
| /** Dependency Container */ | ||
| class InternalDependencyContainer implements DependencyContainer { | ||
| private _registry = new Registry(); |
There was a problem hiding this comment.
Should have probably done this in a separate PR. But the leading _ for private fields isn't really recommended by anyone these days. removing
MeltingMosaic
left a comment
MeltingMosaic
left a comment
There was a problem hiding this comment.
Looking at this, I think we might need to add something to allow stateful factories to dispose themselves as well - for example, instanceCachingFactory won't dispose its retained instance.
| ``` | ||
|
|
||
| # Disposable instances | ||
| All instances create by the container that implement the [`Disposable`](./src/types/disposable.ts) |
There was a problem hiding this comment.
Nit: spelling "created"
| expect(bar.disposed).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("disposes all instances of the same type", () => { |
There was a problem hiding this comment.
Question - what happens for array-type (@injectAll type) resolutions - do we resolve each element in the array?
| } | ||
| } | ||
|
|
||
| it("renders the container useless", () => { |
There was a problem hiding this comment.
It may be useful to have a UT that ensures that register() and resolve() also fail.
| return childContainer; | ||
| } | ||
|
|
||
| public dispose(): void { |
There was a problem hiding this comment.
Maybe this could be an async method.
Take for example https://www.npmjs.com/package/redis QUIT command
3 participants
Closes #127
cc: @mikl