Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request - Integrate `provider` package. #354

Closed
larssn opened this issue Jun 18, 2019 · 17 comments

Comments

@larssn
Copy link

commented Jun 18, 2019

Is your feature request related to a problem? Please describe.
So we have more general requirements from dependency injection than what BlocProvider can do. It's a bit of a pain point that BlocProvider only allows it's dependency to inherit from Bloc, meaning that if I have other non-bloc services, I have to use a different DI package; which we do - the provider package. But that also means we have basically 2 identical DI layers. Also, provider has MultiProvider which is akin to BlocProviderTree.
There are many similarities.

Describe the solution you'd like
My preferred solution would be for this package to embrace the provider package: less redundancy in the community packages, and less confusion.

Describe alternatives you've considered
I see no alternatives besides using both packages.

Thanks for reading, and thanks for a good package!

@timrijckaert

This comment has been minimized.

Copy link

commented Jun 18, 2019

I approve this message.
We too use provider package as ioc and only use the BlocBuilder widget.

@warriorCoder

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

Why don't you guys petition the maintainer of the Provider package to write an extension similar to https://pub.dev/packages/flutter_bloc_extensions? Otherwise what you're asking for is for something with a narrow scope to become more bloated just because?

Or may I suggest you fork bloc and strip out the BlocProvider and then use the Provider package for your DI and BlocBuilder to build up your blocs?

As one who loves using this package, I'm concern that the request put forth will needlessly bloat apps and code bases where the need for another Provider isn't there. So why should I bloat my codebase when I only need BlocProvider because you guys want something that handles everything?

@felangel

This comment has been minimized.

Copy link
Owner

commented Jun 19, 2019

Hi @larssn 👋
Thanks for opening an issue and for providing that feedback!

I have discussed this with the team multiple times and have spoken with the maintainer of provider and we have decided that we currently do not wish to add a dependency on provider for the following reasons:

  • Provider has a very broad scope and promotes mutable state which violates the bloc library paradigm
  • In most cases (in my experience), all developers want is a generic InheritedWidget that looks something like this which doesn't seem to justify importing an entire library.

That's not to say that we will never have a dependency on provider but at this point in time we don't feel that it's the right decision.

Hope that makes sense and I really appreciate the feedback and support. Cheers!

@felangel felangel self-assigned this Jun 19, 2019

@felangel felangel added this to In progress in bloc Jun 19, 2019

@felangel felangel closed this Jun 20, 2019

@felangel felangel moved this from In progress to Done in bloc Jun 20, 2019

@rrousselGit

This comment has been minimized.

Copy link

commented Jun 23, 2019

I have discussed this with the team multiple times and have spoken with the maintainer of provider and we have decided that we currently do not wish to add a dependency on provider for the following reasons

I never agreed on that, I've been suggesting that flutter_bloc should depend on provider myself.

At this point, flutter_bloc is just copying provider with different class names.

I've been fair and purposefully delayed multiple features on provider for a potential partnership with flutter_bloc.
But I wasn't even made aware of that issue, nor the decision. Not cool.

promotes mutable state which violates the bloc library paradigm

It does not. It works with both. ChangeNotifier is just one aspect of the library, that the community asked for.


provider is purposefully generic and light enough for other alternatives to be built on the top of it.

Having flutter_bloc just forking all features from provider misses the point.

I'll add that, from my experience on StackOverflow / Slack, the flutter_bloc binding has multiple issues that lead the community into common traps. Especially with the latest ImmutableProvider.

@larssn

This comment has been minimized.

Copy link
Author

commented Jun 23, 2019

I'll add that, from my experience on StackOverflow / Slack, the flutter_bloc binding has multiple issues that lead the community into common traps. Especially with the latest ImmutableProvider.

Could you elaborate on that?

@rrousselGit

This comment has been minimized.

Copy link

commented Jun 23, 2019

There's an entire field of missed opportunities by requiring a BlocBuilder to work.

  • ImmutableProvider doesn't handle updates.
  • It's impossible to read the current value of a "bloc" outside of build
  • It's impossible to make a "bloc" read-only
  • it's not straightforward to build a "bloc" from other blocs &/or immutable values
  • Why would we need both BlocProviderTree and ImmutableProviderTree?
  • it's not easy to migrate to/out of this library since the inherited widgets are specific to this package.

Using provider, all of these would be solved. Part of the reason is because provider works with literally anything thanks to a slightly different API.

Instead of:

final bloc = BlocProvider.of<MyCounterBloc>(context);

return RaisedButton(
  onPressed: bloc.increment,
  child: BlocBuilder(
    bloc: bloc,
    builder: (_, MyCounterState count) => Text(count.value.toString()),
  ),
);

A flutter_bloc binding using provider would have:

final bloc = Provider.of<MyCounterBloc>(context);
final count = Provider.of<MyCounterState>(context); // or using Consumer<MyCounterState>

return RaisedButton(
  onPressed: bloc.increment,
  child: Text(count.value.toString()),
);

The huge difference is that:

  • MyCounterBloc and MyCounterState can be listened independently from each other.
    Which means that MyCounterBloc could be private, and effectively make the value read-only.

  • We can obtain MyCounterState through any widget life-cycles. Including didChangeDependencies:

    void didChangeDependencies() {
      super.didChangeDependencies();
      print(Provider.of<MyCounterState>(context)); // executed whenever the state changes.
    }
  • We can use ProxyProvider to combine blocs &/or immutable values:

    BlocProxyProvider<MyCounterState, MyTranslationsBloc>(
      initialBuilder: (_) => MyTranslationsBloc(),
      builder: (_, counter, translations) => translations
        ..dispatch(CounterChanged(counter)),
      child: ...,
    )

    this snippet properly updates MyTranslationsBloc whenever MyCounterState updates.
    MyCounterState could be anything, not necesseraly a value from Bloc.

That's just a part of the arguments. But these are the most obvious ones.

@larssn

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

Thanks for the clarification Remi. Is it far fetched to claim that your new BlocProxy can do basically the same as this package? If so, I'd simply opt us out of this one.

@rrousselGit

This comment has been minimized.

Copy link

commented Jun 24, 2019

Is it far fetched to claim that your new BlocProxy can do basically the same as this package?

It's not something that exists in provider. It'd be up to flutter_bloc to create it, as part of the provider+bloc binding.

@jorgecoca

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2019

Hi everyone! 👋
I work with @felangel on the Core team, and I'd like to add some details for everyone to understand our approach:

Felix has brought the topic of integrating with Provider into flutter_bloc many times to our team, even before Google I/O. @rrousselGit you are doing a FANTASTIC job with provider! It is truly outstanding what you are doing, and all the good things you are giving back to the community! 💯

That said, every time we have evaluated the integration, we (as a team at BMW) have decided that we do not see fit by having provider integrated yet into flutter_bloc (maybe in the future, who knows, you can't never tell). Both provider and flutter_bloc share some functionality, but for that we have decided to only depend on InheritedWidget, and nothing else. Given that these libraries (provider and flutter_bloc) are foundational building pieces for Flutter applications, it is our point of view at this moment that the less dependencies we use in the core library (flutter_bloc), the better, so we can have more control over what we deliver (bugs/feature requests/deprecations...). Are we making the right call? So far, we are happy with the feedback received from the community, and our internal feedback and benchmarks.

We would like to keep this conversation open, healthy, positive, and productive. We are always evaluating technology, and trying to do our best to satisfy everyone. Not only provider has been evaluated, there are other many other tools out there that we have decided not to integrate, such as equatable, a package that we have written as well.

We are building flutter_bloc to be extensible and modular (as this other package proves: flutter_bloc_extensions), so the community always have the choice to build an extension using provider if they see fit, or move it into flutter_bloc if the demand is high from the community. I think this is the most important part to understand: provider could be integrated into flutter_bloc via plugins/modules. This is the type of integration that we would love to see: a good, strong catalog of plugins built on top of flutter_bloc that can satisfy all kinds of developers.

Thank you everyone for participating! You are a fantastic community!

@larssn

This comment has been minimized.

Copy link
Author

commented Jun 25, 2019

I'm still missing the basis for the decision that provider shouldn't be included in this package. We've ended up with something less flexible, more rigid. Someone will end up forking your repo, strip out all the redundancy, which will basically simplify it. If that happened then that repo would probably end up becoming more popular than this one. Fragmenting the community like that is unnecessary and avoidable.

I'm a strong believer in the wording "Simplicity is the ultimate sophistication", and I don't wish to see neither bloc nor flutter_bloc get overengineered. I'm still not sure what role the new ImmutableProvider is supposed to fill. What is inside a provider should be up to the programmer, be it something immutable or not, no? The point I'm trying to make is simply, please please keep it simple.

Ps. I should also say thank you for sharing BMW's code with the community. 👍

@rrousselGit

This comment has been minimized.

Copy link

commented Jun 25, 2019

I'm still missing the basis for the decision that provider shouldn't be included in this package.

Since the decision of not using provider has been made official, I'll start my own provider+flutter_bloc binding (similarly to how there's a mobx binding in progress).

I agree with you that having two packages essentially doing the same thing is bad, and I want to avoid competing with that package as much as possible.
Sadly I don't see any other way.

@felangel

This comment has been minimized.

Copy link
Owner

commented Jun 25, 2019

@larssn, sorry if I was unclear but the basis for the decision is as follows:

Provider's scope is extremely broad and is even being promoted as a State Management Solution as of recently.
The package description was updated from

"Syntactic Sugar around InheritedWidgets"

to

"A mixture between dependency injection (DI) and state management, built with widgets for widgets".

I personally don't understand why provider is trying to be more than a DI library but I respect the author's decisions and try to stay out of it.

Futhermore, provider promotes the use of ListenableProvider, ChangeNotifierProvider, ValueListenableProvider, etc... which do not fit into the flutter_bloc paradigm.
Had provider remained a simple wrapper around InheritedWidget, we would likely not be having this discussion.

I agree that simplicity is key; however, in my opinion, introducing provider would complicated things quite a bit. We would now have to take an API that is trying to solve every problem and put it in a library with a very focused scope.

The part of flutter_bloc which overlaps with provider is the DI. flutter_bloc has very clear specifications which allow for a simplified api. Issues like this will not happen in flutter_bloc because the library is not trying to solve everything allowing us to hide much of the complexity.

As I mentioned before, this was not a hasty decision; we've had many discussions around the topic and carefully considered both sides.

Lastly, as @jorge mentioned, there is nothing currently stopping developers from adding provider as a dependency and using it with flutter_bloc. I'm not sure why you're making it seem like that is not an option.

The change to introduce ImmutableProvider was actually requested by the community several times by individuals who did not want to add another dependency to their project just for DI of repositories (~30 lines of code). I'll just add that we use our own library every day at BMW and have no reason to "over-complicate" or "over-engineer" the library.

I encourage @rrousselGit and others to create their own bindings to fit their needs and hope I clarified our perspective.

@larssn

This comment has been minimized.

Copy link
Author

commented Jun 25, 2019

@larssn, sorry if I was unclear but the basis for the decision is as follows:

Provider's scope is extremely broad and is even being promoted as a State Management Solution as of recently.
The package description was updated from

"Syntactic Sugar around InheritedWidgets"

to

"A mixture between dependency injection (DI) and state management, built with widgets for widgets".

I personally don't understand why provider is trying to be more than a DI library but I respect the author's decisions and try to stay out of it.

Well, to be fair, it is DI. That's all the provider pattern is in a nutshell: IOC. And I think it is a quite simple one at that.

The change to introduce ImmutableProvider was actually requested by the community several times by individuals who did not want to add another dependency to their project just for DI of repositories (~30 lines of code). I'll just add that we use our own library every day at BMW and have no reason to "over-complicate" or "over-engineer" the library.

And this kind of redundancy is exactly the problem in my eyes: re-inventing the wheel.
But I understand your position at least, and I suppose I can get where you're coming from in not wanting to include a library than can do more than you need (maybe you can understand where I'm coming from, when I need two libraries with a big overlap in functionality).

I respect your decision, even though I disagree with it. 😉

Thank you for your time.

edit: Forgot to mention, Google inadvertently advertised for provider at the last Google IO: https://www.youtube.com/watch?v=d_m5csmrf7I
They gave it their stamp of approval by doing so, so it's probably not gonna go away, which was why I was hoping for some collaboration.

@basketball-ico

This comment has been minimized.

Copy link

commented Jun 25, 2019

@larssn Hello

can you said me please what are you using of provider library when you develop apps with the bloc library?

At the moment, when I use bloc library, I only used provider class for DI instead of write my own InheritedWidget

For this reason I prefer that flutter bloc library has its own DI with simplified API for bloc library users.

Thanks

@larssn

This comment has been minimized.

Copy link
Author

commented Jun 26, 2019

@basketball-ico
We're using provider for all our DI, as it doesn't really make sense to use BlocProvider for blocs and Provider for non-blocs. They do the same thing anyway. So we've opted to just use Provider for all.

@larssn

This comment has been minimized.

Copy link
Author

commented Jul 11, 2019

So I see you decided to do this anyway? cc340a0

Thats great, thanks for listening! 👍

@felangel

This comment has been minimized.

Copy link
Owner

commented Jul 12, 2019

@larssn yup after lots of discussions with @rrousselGit 👍

Thanks to you and everyone who gave their input. I really appreciate it! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
7 participants
You can’t perform that action at this time.