tappeddev / injector

Simple dependency injection for Dart. 💉
Apache License 2.0
77 stars 7 forks source link

Feature request: tear-down logic #32

Closed f3ath closed 3 years ago

f3ath commented 3 years ago

Some services may need to be closed properly when the application terminates (e.g. HTTP clients). It seems convenient to have this "tearDown" logic in a separate function and to call this function before the app exists. On the other hand, when the app terminates, there is no guarantee that the service has been used (and therefore instantiated) at all. It seems awkward to instantiate a service just to close it immediately after, especially when the instantiation is expensive. I'd like to have a way to conditionally execute some logic if and only if the service has been instantiated already. So far I imagine several options.

  1. A one-off method. The callback will be executed only if the service is already instantiated (for singletons) and it never executes for providers. For singletons, it also should remove the instance, since the tear down logic will most likely put the service in an invalid state.

    class Injector {
    Future<void>tearDown<T>(FutureOr<void> Function(T service) tearDown, {String dependencyName = ""});
    }
  2. A more centralized approach. When registering a singleton, provide the tear down logic. Create the "tearDownAll" method which would execute tear-down procedures for all instantiated singletons.

    class Injector {
    void registerSingleton<T>(
    Builder<T> builder, {
    bool override = false,
    String dependencyName = "",
    FutureOr<void> Function(T service)? tearDown, // optional tear-down logic
    }) { ... }
    
    Future<void> tearDownAll() {
    // iterate over singletons
    // execute the tear-down procedure (if any)
    // remove the instance
    }
    }

Naming can def be better, I'm not good at it. Please let me know what you think. I'm willing to submit a PR.

JulianBissekkou commented 3 years ago

I always thought about having a way to clear specific parts of the injector. We currently have a method to clear all dependencies so your second approach would mix great I guess.

Regarding the naming I would probably call it "onClear". What do you think?

Every Factory class would need a new method that would call the onClear callback.

Should be straight forward.

(I am currently on my phone and not on my computer so it might take some time to answer)

f3ath commented 3 years ago

I can't think of a use case when one would need to remove one or more dependencies, so I may be missing something. But it seems that clearing the container or removing a dependency are not exactly the same as shutting down a service.

Another argument against conflating the two operations is that clearing the container seems to always be synchronous. Shutting down, on the other hand, may require async operations, so must return a Future or a Stream. Therefore, it is not possible to fit the shut down into "clearAll()" and "removeByKey()" without breaking the existing API.

P.S. Regarding the naming, how about:

f3ath commented 3 years ago

Actually disregard this. 🤦🏻‍♂️ I just realized that what I actually need is simply a T? getCached<T>() method which would return a cached instance (if any).

JulianBissekkou commented 3 years ago

Yes, when I read through your last comment I thought about this well. This would be a more generic use case. 👍 Two simple steps would be required to implement this feature.

  1. Write a T? _getFactory<T>() method.
  2. Add the cachedValue field to the SingletonFactory so you can return the value.

Feel free to submit a PR 👍