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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Discussion] Maintaining Provider Dependency #712
Comments
extra dependencies are always a problem, I think it's better to just use InheritedWidget (which provider extends) instead of having extra overhead |
The v4 of provider will have lazy-loading. That's likely something In the end, it's your call though. For what it's worth, if your worry is about: class BlocProvider<T extends Bloc<dynamic, dynamic>> extends Provider<T> {
...
} vs: class BlocProvider<T extends Bloc<dynamic, dynamic>>
extends ValueDelegateWidget<T> implements SingleChildCloneableWidget {
...
} then it's a relatively easy fix. One of the goals of the 4.0.0 of provider is to simplify custom providers. As such, if that's what you need, I can make class BlocProvider<T extends Bloc<dynamic, dynamic>> extends InheritedProvider<T> {
BlocProvider({
Key key,
@required Create<T> create,
Widget child,
}) : super(
key: key,
create: create,
dispose: (_, bloc) => bloc?.dispose(),
child: child,
);
BlocProvider.value({
Key key,
@required T value,
Widget child,
}) : super.value(
key: key,
value: value,
child: child,
);
static T of<T extends Bloc<dynamic, dynamic>>(BuildContext context) {
try {
return Provider.of<T>(context, listen: false);
} catch (_) {...}
}
} |
I'm sure there's plenty of people using both packages in their apps, myself being one of those. I think being able to pass |
@RollyPeres can you please elaborate on how lazy loading would benefit flutter_bloc? Also, I鈥檇 love to understand how you use both together. What do you provide with provider aside from blocs when using flutter_bloc? |
@felangel I believe lazy loading would be great for global blocs which does't really need to be instantiated when app starts but later on when they might be needed. As for your second question, I mostly provide Firebase streams, since I'm not a fan of manually listening to streams unless it's absolutely necessary. And
|
In the end, the main goal of Aka providers having two constructors, one stateful with a The other part is to help maintainers to respect such convention (as having two constructors with different logics can be painful), so that as many people as possible respect that convention. So I won't die if Although I'm fairly certain that folks will want lazy-loading 馃槢 |
We provide services that encapsulate non-flutter related business tasks. This could be a service that provides a low level printing layer via TCP, which I might need to inject into other services or BLoCs. We use Provider for this, as it offers a clean way of doing DI without any side effects. Regarding lazy loading, in my experience it's not the end all be all achievement; there's always a catch. It's good for reducing load times, but can make the app feel sluggish until it is "warm". I hope you make it optional. 馃檪
I will literally die, if you ditch |
@RollyPeres thanks for the additional information. Regarding lazy loading, I think that can be easily supported regardless of the dependency on @RollyPeres, @larssn
Can you provide a bit more detail regarding why you decided to use this approach rather than the one outlined in the architecture docs? Thanks! |
@felangel
I hope that clarifies how we're doing things a little. |
My two cents. We too use the About the proposed architecture Just like @larssn said we also have helper/service classes for this which we inject in multiple BLoC's that purely consists of reusable functions. Easy to unit test in isolation, ... Maybe a bit of offtopic, but this sometimes looks weird when we need to provide this dependency. |
@felangel I'm always pro separation of concerns and layering your app and whatnot, but it all depends on your use case. For me doing that would sometimes mean I would have to manually subscribe to streams coming from firebase and that's something I don't want to. I prefer not to break the pipe, if you know what I mean. So sometimes I prefer to transform data from the incoming stream to whatever I need, without involving BLoC. |
Hey everyone thanks for the awesome feedback! I'm going to try my best to respond to everyone's comments and am looking forward to hearing your thoughts 馃憤
In that case I would recommend exposing the business logic to the bloc via a repository and providing that repository using
I totally understand this concern and it was actually one of the major reasons for creating the bloc library. You shouldn't need to interact directly with Rx very often when using the bloc library because the whole goal was to abstract the Rx while still allowing you to write reactive code. Is there anything in particular that you are concerned about in terms of complexity when using the bloc library? At the end of the day it should just be simple logic that maps events to states (especially if you have separated your architecture into the 4 aforementioned layers).
I'm not sure in this case what the difference between a service and a repository is. Unless I'm misunderstanding, you still want to maintain separation between the presentation and data layers so in this case your services could be parts of repositories. Just out of curiosity, is there a particular reason why you've decided against DDD? Thanks!
It would be very useful if you could elaborate and provide some specific cases where you felt the architecture was too limited? Also, if you are sharing stateless business logic I'm not sure I understand why you need to use int sum(int x, int y) => x + y; import 'sum.dart';
BlocProvider<MathBloc>(
builder: (_) => MathBloc(sum),
)
I agree with you and that's intentional because
If that was unclear, I'm more than happy to go back through the docs and update them as needed. Please let me know if it was just a misunderstanding or if you still feel like there are limitations of this approach which require directly providing data providers to the UI. @RollyPeres
I'm not sure I understand your reasoning for this. By directly consuming the stream from Firestore you're tightly coupling your UI to your raw entities. If you wanted to share the data across multiple applications it would become very difficult to maintain. Also, is there a particular reason why you're hesitant to subscribe to a
Can you elaborate? I just want to note that
The goal of this discussion was for me to voice my concerns about the dependency on Hope that clarifies things and again I really appreciate the feedback and am looking forward to your responses. Thanks everyone! 馃檹 |
Not really, you can map your entity to a different model if needed. If you replace your incoming firebase stream to something else, you still need to deal with backend specific implementations. I'm well aware of how to manually listen to streams, I just prefer not to do something that would only add noise and no real gain to my app. Why do you think sharing some stream source would be difficult ? At the end of the day, if someone goes with your repository suggestion it means you will manually get the data from the firebase stream then convert it to some model. How is that different from exposing a stream that you map it to some model using a stream operator ? I'm totally up for repo layer when you deal with futures and standard API's. But when the data source is a stream, I think you only lose the goodies streams have to offer by manually subscribing instead of taking advantage of operators. I guess is just a matter of preference. You can easily share a stream and even map it to different models if needed, without really needing another layer. It all depends on what you are trying to achieve I guess. |
Yes. It's something many people get wrong, because they don't understand it's many many concepts. There's a lot in DDD I don't understand myself, so I'd rather not force people down that road. I prefer my models to be anaemic 馃榿 I like that BLoC is pretty open ended: it's doesn't force my hand to adopt DDD. |
@RollyPeres in what part of your application do you do the Stream transformation? In my opinion, it's more about abstraction; if you're transforming the raw Firestore stream in your UI layer you no longer have any layer of abstraction which means your UI is tightly coupled to Firestore. If your application blows up in terms of users, for example, and you decide you want to switch to a different DB to save on cost you will end up having to rewrite a lot of code. @larssn can you please provide any specific examples of concepts that are unclear or difficult to understand in your experience? Ultimately, even though you're both using bloc in a way which diverges from the recommended architecture the decoupling of provider from bloc shouldn't have a large impact since the API will remain the same. You will just need to add provider as a dependency to you return MultiProvider(
providers: [
// * Independent services
Provider<DbService>.value(value: DbService()),
Provider<AuthService>.value(value: AuthService()),
Provider<PostsService>(
builder: (_) => PostsService(),
dispose: (_, PostsService value) => value.dispose(),
),
// * Dependent services
ProxyProvider<DbService, SeedService>(
builder: (_, DbService dbService, SeedService __) =>
SeedService(dbService: dbService),
),
// * UI providers
BlocProvider<AppBloc>(builder: (_) => AppBloc()),
StreamProvider<User>(
builder: (BuildContext context) =>
Provider.of<AuthService>(context, listen: false).user,
),
],
child: child,
); into return MultiProvider(
providers: [
// * Independent services
Provider<DbService>.value(value: DbService()),
Provider<AuthService>.value(value: AuthService()),
Provider<PostsService>(
builder: (_) => PostsService(),
dispose: (_, PostsService value) => value.dispose(),
),
// * Dependent services
ProxyProvider<DbService, SeedService>(
builder: (_, DbService dbService, SeedService __) =>
SeedService(dbService: dbService),
),
// * UI providers
StreamProvider<User>(
builder: (BuildContext context) =>
Provider.of<AuthService>(context, listen: false).user,
),
],
child: MultiBlocProvider(
providers: [
BlocProvider<AppBloc>(builder: (_) => AppBloc()),
...
],
child: child,
),
); Even the latter won't be necessary if the |
Well I'm talking about DDD, I assume you're referring to the concepts in BLoC? |
@larssn I'd be very interested to understand why you prefer not to follow the architecture outline in the documentation when using bloc so that I may potentially make improvements. |
@felangel As I mentioned earlier, in a previous project, we went overboard with Rx (JS), and while we're all fricking Rx experts at this point from flatmapping our concatmaps, that product ended up being overly complex, and very hard to maintain. So this time, we use streams pretty sparsely. |
I also try to avoid that at all costs. If you need bloc to bloc then you might actually need that logic in a separate bloc provided on top of the others. At the end of the day, you could even
Stream transformation happens in the service, which is nothing but a glorified repository at the end of the day really, which instead of |
@larssn it would be great if you could provide some examples of the scenarios you are describing. Depending on the case, I would argue that logic that is shared between several blocs could be part of the repository layer so you would not need bloc to bloc communication. Also date formatting is a UI specific thing so I would recommend creating a DateFormatter widget like: DateFormatter(
date: date,
builder: (formattedDate) { ... }
), We had the same fear coming from RxSwift so I totally understand your perspective. The bloc library was our way of using Streams in a safe/scalable way. If you are able to provide some scenarios as I mentioned above I will try my best to improve those areas/pain-points. Thanks! |
@RollyPeres thanks for clarifying! So just for my understanding, you avoid subscribing to streams in blocs and instead prefer to directly transform the stream in a service which you directly consume in your widgets via a StreamBuilder? All this wouldn't be an issue for you if there was a better way to transform/manipulate a stream within a bloc without having to subscribe? |
@felangel no problem at all! I do avoid manually subscribing to streams and I do transform streams inside a service, but I don't actually use a |
Broadcast stream support wouldn't be bad. I made an issue about that recently, since I thought it was supported. @felangel I think we're mostly debating semantics. Our current approach doesn't really display any pain-points, it's just a slightly different way of doing things. :-) One could argue it's just a different way of naming things. It's just that in my view, a repository should be fairly basic, and only handle data access, and not complex business logic. Our repos transforms a Firestore document into Dart object instances (built_value/built_collection for immutability), handles save operations and thats it. |
@rrousselGit, @RollyPeres, @larssn, @timrijckaert, @bigworld12 would love you feedback/opinions regarding #765 |
@netantho I don't think so because this PR is related to provider as a dependency not rxdart 馃憤 |
Hopefully you'll find some inspiration here: https://youtu.be/_ISAA_Jt9kI |
Hey everyone! 馃憢
I am becoming concerned about the dependency on provider due to the increase in complexity. After each upgrade I find myself questioning whether it makes sense for the bloc library to continue to maintain the dependency on provider and wanted to start this discussion in order to see what everyone's thoughts are.
A few specific examples that come to mind include:
Provider
from being used to provideStreams
. Before bloc v1.0.0BlocProvider
was able to simply extendProvider
which made the implementation extremely straightforward but after v1.0.0 (when bloc extendedStream
)BlocProvider
had to be refactored to extendsValueDelegateWidget<T>
and implementSingleChildCloneableWidget
with a manualbuild
override to use anInheritedProvider
. In this case, the implementation went from:to
You might argue that we didn't add that much code but it wasn't immediately obvious to me what
ValueDelegateWidget
was for so I had to spent a significant amount of time understanding the implementation details of provider in order for me to feel comfortable refactoring the code.buildWithChild
introduced along withSingleChildStatelessWidget
, etc... with the dependency on nested inv4.0.0-dev
. While I like the idea, in practice I'm not happy with the implementation and there are several major limitations including not being able to use it as amixin
and also having to deal with a potentiallynull
child whenbuildWithChild
is called (unless I'm misunderstanding how to use it). I personally prefer the previous, simpler implementation because it was easy to understand and didn't involve customWidget
andElement
implementations.flutter_bloc only requires a small subset of the functionality provider offers and I feel it would be much easier/less time-consuming to maintain the flutter_bloc library if I didn't have to do a deep-dive of the provider implementation after every major change in order to understand how it will impact the bloc library.
The goal of this issue was just to start a discussion and see what everyone thinks. I'm more than happy to continue to maintain the dependency, but I just want to make sure that the cost of maintenance is justified by the benefit to the community.
cc @rrousselGit, @brianegan, @larssn, @timrijckaert, @jorgecoca
The text was updated successfully, but these errors were encountered: