feat(flutter_bloc): Expose optional dispose parameter to RepositoryProvider

[Luckey-Elijah]

Description

Repositories often have HTTP clients, stream controllers, or other objects that need to be disposed, canceled, or closed. Given a rule of thumb: whoever is creating the object should dispose the object.

Given that many use cases with flutter_bloc's RepositoryProvider create a repository at a global/top-level, the provider here should also provide a way top dispose a create repository.

Desired Solution

Current implementation:

class RepositoryProvider<T> extends Provider<T>
    with RepositoryProviderSingleChildWidget {
  RepositoryProvider({
    Key? key,
    required Create<T> create,
    Widget? child,
    bool? lazy,
  }) : super(
          key: key,
          create: create,
          dispose: (_, __) {},
          child: child,
          lazy: lazy,
        );
  ...
}

Proposed change:

class RepositoryProvider<T> extends Provider<T>
    with RepositoryProviderSingleChildWidget {
  RepositoryProvider({
    Key? key,
    required Create<T> create,
    Widget? child,
    bool? lazy,
    void Function(BuildContext, T)? dispose, // adding this optional field
  }) : super(
          key: key,
          create: create,
          dispose: dispose ?? (_, __) {}, // adding this call to super constructor
          child: child,
          lazy: lazy,
        );
  ...
}

Alternatives Considered

I have seen several examples of a Bloc or Cubit taking the responsibility to execute a dispose, cancel, or close method on a repository. For example: flutter_login authentication bloc. Or a repository never has a repository client closed at: fluttersaurus #10

Additional Context
N/A

[felangel]

Hi @Luckey-Elijah 👋
Thanks for opening an issue!

This is a duplicate of #2085. Closing for now but feel free to comment if you have additional comments/questions. Repositories should not be coupled with the application lifecycle and generally should not manage the lifecycle of things like http clients.

[Luckey-Elijah]

Thank you, I would mostly agree with what is described here and the other referenced issue. However:

Repositories .. should not manage the lifecycle of things like http clients.

Yet in a majority of examples I have found, the repository seems to take responsibility for the lifecycle of a client (please see provided examples of flutter_login and fluttersaurus).

Could you provide a reference to an example that would inject some HTTP client (or any other "disposable" object) into a repository and properly dispose the client.

Thanks, again.

[felangel]

Thank you, I would mostly agree with what is described here and the other referenced issue. However:

Repositories .. should not manage the lifecycle of things like http clients.

Yet in a majority of examples I have found, the repository seems to take responsibility for the lifecycle of a client (please see provided examples of flutter_login and fluttersaurus).

Could you provide a reference to an example that would inject some HTTP client (or any other "disposable" object) into a repository and properly dispose the client.

Thanks, again.

In both of those cases the repositories are global and the http client instance is injected into the repo so it’s not the repository’s responsibility to close the instance.

Also, in both cases the http client is used globally so the only time it would make sense to dispose it is when the application is detached but I’m also not sure if that’s really necessary since it should be garbage collected when the app has exited.

Is there an example of a global http client being closed that you’re referring to? Thanks!

[Luckey-Elijah]

Thanks for educating me 😄

But I'm not sure I quite follow..

With the fluttersaurus example, there is an inject-able HTTP client, but a client instance is never is actually injected; hence my confusion of the responsibility of the client's lifecycle since DatamuseApiClient constructor ends up creating one anyways. Is this intentional? It seems to me that it still seems to contradict the rule of responsibility.

Is there an example of a global http client being closed that you’re referring to?

I don't have an example in mind, perhaps I can conjure up one if needed.

[felangel]

Thanks for educating me 😄

But I'm not sure I quite follow..

With the fluttersaurus example, there is an inject-able HTTP client, but a client instance is never is actually injected; hence my confusion of the responsibility of the client's lifecycle since DatamuseApiClient constructor ends up creating one anyways. Is this intentional? It seems to me that it still seems to contradict the rule of responsibility.

Is there an example of a global http client being closed that you’re referring to?

I don't have an example in mind, perhaps I can conjure up one if needed.

I’ll take a closer look at fluttersaurus I don’t recall exactly how it was implemented and I’m away from the computer now. Will take a look in a bit and get back to you 👍

[Luckey-Elijah]

Thank you very much!

[narcodico]

@Luckey-Elijah it is indeed using an optional http client which defaults/fallbacks to an internal instance if one is not passed in from the outside. This is a pattern often used since it helps with reducing the DI boilerplate, while also allowing for passing a concrete instance if needed(e.g.: a mocked instance for testing).

As long as you don't need to explicitly call in a dispose method there's not really a need to explicitly instantiate it from the outside, although that's perfectly fine to do, especially to inject that http client instance into multiple api clients.