freshgum-bubbles / typedi

Elegant Dependency Injection in TypeScript and JavaScript.
http://typedi.js.org/
MIT License
53 stars 3 forks source link

Add a (better) way to notify services of container disposal #4

Open freshgum-bubbles opened 1 year ago

freshgum-bubbles commented 1 year ago

Currently, containers are just dropped from the service with no warning: https://github.com/freshgum-bubbles/typedi/blob/ee8a84e3d82ace1b99084848dae673d13bdc4d05/src/container-instance.class.ts#L556-L572

This seems quite strange. Services would surely want some form of notification as to when their container is disposed of.

While a seemingly simple concept, the notion of how individual services are meant to respond to such a notification is ambiguous. For instance, in the case of a database service, is it meant to close down its database connection(s) when the container is disposed? Furthermore, in such a case, should we provide a way for the caller of .dispose or .reset to know when all such asynchronous events have been concluded?

One further challenge is how you would provide an API for such a notification. It would be awkward to make services conform to a standard interface for notification, and any runtime type-checking of notification handler methods could incur runtime errors.

This somewhat ties into https://github.com/typestack/typedi/issues/230:

allow containers to dispose of themselves, destroying all locally registered service

Proposed APIs

Addition of @DisposeService decorator

@Service([])
export class DatabaseService {
  @DisposeService
  async dispose(): Promise<void> { }
}

@Service([DatabaseService])
export class RootService {
  constructor (private database: DatabaseService) { }

  @DisposeService([DatabaseService])
  async dispose (): Promise<void> { }
}

The decorator allows a list of dependencies to be passed. The dependencies are guaranteed not to be disposed before the returned dispose promise is settled.

For example, in the case of RootService, it may want to perform further operations on the database (flushing caches, etc.) before the database is disposed of (which would close the connection). In this case, it can declare that it requires DatabaseService to dispose of the application correctly. As such, it can return a Promise that, until settled, will have an undisposed reference to DatabaseService available.

Use HostContainer

This is already a no-go, but for posterity...

Using this API would mean that tests would have to pass in a container instance for potentially nothing. Also, we'd have to expose an addDisposeListener or the likes onto the class, which feels awkward.


Definitely needs further investigation.

freshgum-bubbles commented 1 year ago

Oh, interesting.

I was wizzing about yesterday and must have missed this in ContainerInstance: https://github.com/freshgum-bubbles/typedi/blob/d970fdc12be09d2a42f5450be411caae41a3f2a4/src/container-instance.class.ts#L723-L741

This seems like a major hack. The service might not be expecting the DI container to be performing such operations in a brittle manner.
As there isn't a decided, explicit way to notify services of container disposal yet, I'll be removing this behaviour in the next revision.

freshgum-bubbles commented 1 year ago

One possible implementation of this could be as follows:

import { Service, OnDispose } from '@typed-inject/injector';

@Service([OnDispose()])
export class MyService {
  constructor (private onDispose: OnDispose) { }

  bootstrap () {
    this.onDispose(() => console.log('being disposed'));
  }
}
freshgum-bubbles commented 1 year ago

A much simpler implementation would be the usage of Symbol.dispose, part of the upcoming ECMAScript Explicit Resource Management proposal.