Closed dalewking closed 4 years ago
Hi @dalewking 👋 Thanks for opening an issue!
I'm not sure I fully understand the issue but the snippets you provided are invalid because class Cubit
does not accept 2 generic type parameters (Cubit<int, Foo>
).
The following seems to work fine and I believe accomplishes what you want:
import 'package:bloc/bloc.dart';
abstract class FooState {
bool get foo;
}
abstract class FooBloc implements Cubit<FooState> {}
class FooStateImpl implements FooState {
bool foo;
}
class MyBloc extends Bloc<int, FooState> implements Cubit<FooState> {
MyBloc() : super(FooStateImpl());
@override
Stream<FooStateImpl> mapEventToState(int event) {/* ... */}
}
Let me know if that helps 👍
I was typing that into GH and not the IDE so yes that should have beenCubit<FooState>
.
The problem though is that FooStateImpl can have additional functionality and you want to be able to access that functionality inside of mapEventToState.
So in my case FooStateImpl is actually an implementation using the freezed library but consider this hand-written example where we want to maintain some additonal state that is not exposed to the view and we want it to be immutable:
class FooStateImpl implements FooState {
const FooStateImpl(this.foo, this.bar);
final bool foo;
final int bar;
FooStateImpl copyWith({bool foo, int bar}) => FooStateImpl(foo ?? this.foo, bar ?? this.bar);
}
so inside of mapEventToState you might want to do something like:
yield state.copyWith(foo: false, bar:5);
But you cannot do that because state is only exposed as the abstract type.
So the only way to declare it is if both the interface that BlocBuilder sees and the one that the Bloc implementation are the same interface. It does not allow the bloc implementation to use a state subclass.
@dalewking check out:
import 'package:bloc/bloc.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
abstract class FooState {
bool get foo;
}
abstract class FooBloc<E, S extends FooState> extends Bloc<E, S> {
FooBloc(S state) : super(state);
}
class FooStateImpl implements FooState {
const FooStateImpl(this.foo, this.bar);
final bool foo;
final int bar;
FooStateImpl copyWith({bool foo, int bar}) =>
FooStateImpl(foo ?? this.foo, bar ?? this.bar);
}
class MyBloc extends FooBloc<int, FooState> {
MyBloc() : super(FooStateImpl(false, 0));
@override
FooStateImpl get state => super.state as FooStateImpl;
@override
Stream<FooStateImpl> mapEventToState(int event) async* {
yield state.copyWith(foo: false, bar: 5);
}
}
void main() => runApp(MyApp());
class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return MaterialApp(
home: BlocProvider<FooBloc>(
create: (_) => MyBloc(),
child: HomePage(),
),
);
}
}
class HomePage extends StatelessWidget {
@override
Widget build(BuildContext context) {
return BlocBuilder<FooBloc, FooState>(
builder: (context, state) {
return Text(state.foo.toString());
},
);
}
}
In the above example, BlocBuilder
is only referencing the abstract type and so the view cannot access parts of the implementation like copyWith
.
The Bloc
implementation itself (MyBloc
) uses the concrete implementation internally and has access to copyWith
and bar
.
Let me know if that helps 👍
@dalewking closing this for now but feel free to reopen it and/or add additional questions/comments if you feel the answer is not what you were looking for, thanks!
What the heck? What is your justification for closing this and no I am not permitted to reopen it.
Your second piece of code is even worse. You have completely eliminated ALL hiding of implementation!
Your view in the second piece not only has the view directly referencing the state implementation, but also the bloc implementation.
Since you apparently need some education on the subject: https://stackoverflow.com/questions/2697783/what-does-program-to-interfaces-not-implementations-mean#:~:text=It%20means%20that%20you%20should,instead%20of%20the%20implementation%20directly.&text=So%2C%20your%20code%20knows%20about,is%20defined%20on%20this%20contract.
I should be able to have a view that uses BlocBuilder that is injected with an abstract class that represents the interface to the Bloc implementation. I prefer to not even expose the add method and the event type and use abstract methods whose implementation calls the add event.
So let me try to explain this once more with a real example:
@freezed
abstract class LandingState with _$LandingState {
LandingState._();
factory LandingState({bool isLoading, bool showLoginButton}) = _LandingState;
}
abstract class LandingModel implements Cubit<LandingState> {
void checkLoginStatus();
void doLogin();
void dispose();
}
@injectable
class LandingView extends StatefulWidget {
LandingView(this.model);
final LandingModel model;
@override
_LandingViewState createState() => _LandingViewState();
}
and in the BlocBuilder I pass the model.
In another part of the code base I define the implementation of this Model:
@Injectable(as: LandingModel)
class LandingBloc extends Bloc<_Event, LandingState> implements LandingModel {
Your second piece of code says scrap the abstract model class and instead reference the implementation directly. Sorry that is not an answer.
So the only issue with my code is that I am exposing the LandingState as the concrete class instead of also letting that just be an abstract class only defining the interface of what I want the view to see. But I cannot do that because Cubit is not covariant on the state type.
Guess I will have to try going the PR route to show you how this should be done. I don't think it is possible to make Cubit covariant so may require extracting an abstract class above Cubit that is really just a Stream subclass that adds a getter for state.
@dalewking I'm happy to reopen this issue but I would appreciate not having such a negative/condescending tone.
I apologize and corrected the above snippet:
import 'package:bloc/bloc.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
abstract class FooState {
bool get foo;
}
abstract class FooBloc<E, S extends FooState> extends Bloc<E, S> {
FooBloc(S state) : super(state);
}
class FooStateImpl implements FooState {
const FooStateImpl(this.foo, this.bar);
final bool foo;
final int bar;
FooStateImpl copyWith({bool foo, int bar}) =>
FooStateImpl(foo ?? this.foo, bar ?? this.bar);
}
class MyBloc extends FooBloc<int, FooState> {
MyBloc() : super(FooStateImpl(false, 0));
@override
FooStateImpl get state => super.state as FooStateImpl;
@override
Stream<FooStateImpl> mapEventToState(int event) async* {
yield state.copyWith(foo: false, bar: 5);
}
}
void main() => runApp(MyApp());
class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return MaterialApp(
home: BlocProvider<FooBloc>(
create: (_) => MyBloc(),
child: HomePage(),
),
);
}
}
class HomePage extends StatelessWidget {
@override
Widget build(BuildContext context) {
return BlocBuilder<FooBloc, FooState>(
builder: (context, state) {
return Text(state.foo.toString());
},
);
}
}
The view is only provided with FooBloc
(the abstract bloc) and FooState
(the abstract state).
The bloc accepts the abstract state (FooState
) and internally casts it to the concrete implementation.
Maybe I need further education but I fail to understand why you need a LandingModel
which extends Cubit
. The state exposed by blocs/cubits should be the model that the UI consumes so if you wish to have a view which does not have access to the bloc you can just inject the state directly into the view itself:
class MyPage extends StatelessWidget {
@override
Widget build(BuildContext context) {
return BlocBuilder<MyBloc, MyState>(builder: (context, state) => MyView(model: state));
}
}
class MyView extends StatelessWidget {
const MyView({Key key, @required this.model}) : super(key: key);
final MyState model;
@override
Widget build(BuildContext context) {
// use model without having access to the implementation details of the bloc/cubit
}
}
Thoughts?
The purpose is that you want the view to know as little as possible about the implementation it is talking to. It should only know what methods it needs to call. The implementation should be somewhere else provided by dependency injection. It is the principle of coding to interfaces not implementation.
The reason for Cubit over Bloc is that I do not want the view to even know about the events the bloc takes in. The model defines the method signatures that the view calls which get translated into adding events. Cubit is the most abstract interface that BlocBuilder can accept. I think there is virtue in extracting an interface above Cubit that specifies only what BlocBuilder needs, which is essentially a Stream with a state property.
But as I am discovering Dart has a pretty broken type system and I am not sure this is even possible now with Dart other than sidestepping type safety. This would be trivial in Kotlin. I will continue to research if this can be done in Dart.
Yes I understand but you can use Widgets to accomplish this as well. Suppose you had "Container" widgets and "View" widgets then you can make it the responsibility of the container widget to create/interact with the bloc/cubit and inject the relevant information into the "View" widget.
Also, as far as I know declaration-site variance is still experimental in Dart so it wouldn't be feasible to introduce into the library at this point.
Also, as far as I know declaration-site variance is still experimental in Dart so it wouldn't be feasible to introduce into the library at this point.
It also wouldn't be enough, since you can't even do it with just Stream class. Stream<FooState>
and Stream<FooStateImpl>
are not type compatible.
But I must thank you that you did point me to the solution to at least get what I want on the view side. That answer was to do this with the Bloc:
@Injectable(as: LandingModel)
class LandingBloc extends Bloc<_Event, LandingState> implements LandingModel {
@override
_LandingState get state => super.state as _LandingState;
Technically it is not type safe as the cast will fail if you added a different implementation of LandingState, but at least that is confined to the implementation of the Bloc.
I would still argue that there should be an abstract class above Cubit that looks something like this:
abstract class StateStream<T> extends Stream<T> {
T get state;
}
abstract class Cubit<State> extends StateStream<State> {
and that should be the type of the parameter to BlocBuilder since that is all BlocBuilder needs, but that doesn't solve the variance issue. It would make it slightly easier to create a mock for testing since there is less to mock.
It also wouldn't be enough, since you can't even do it with just Stream class. Stream
and Stream are not type compatible.
Yes I agree.
But I must thank you that you did point me to the solution to at least get what I want on the view side. That answer was to do this with the Bloc
Glad I was able to help but yes as you pointed out it's not type-safe and can break at runtime if you use a different implementation.
I would still argue that there should be an abstract class above Cubit that looks something like this:
What would be the benefit? You mentioned it would be slightly easier to mock but the effort to mock in either case is the same when using mockito
since you only need to stub the API surface which is being exercised.
It would make BlocBuilder a bit more universal in that it does not need to be tied to Bloc or Cubit at all. It would work with anything that supported just this simplified interface. You have BlocBuilder requiring a Cubit, but it doesn't need anything from Cubit except that it is a Stream and has a state property.
Not a huge advantage, but is better design. It is sort of like the difference between declaring a List parameter when all you really need is an Iterable.
Glad I was able to help but yes as you pointed out it's not type-safe and can break at runtime if you use a different implementation.
Actually since I declared mapEventToState like this it actually is pretty type-safe:
@override
Stream<_LandingState> mapEventToState(_Event event) async* {
It would make BlocBuilder a bit more universal in that it does not need to be tied to Bloc or Cubit at all.
Yes but my intention was to restrict it to bloc/cubit since that is what it was implemented to be used with. Just because the explicit API surface area it is interacting with is smaller than the surface area exposed by bloc/cubit doesn't mean it makes sense to create another, more generic interface. For example, until recently blocs and cubits emitted the previous state upon subscription (like a BehaviorSubject) and recently this behavior was changed. While technically the API surface didn't change, the behavior did and it would certainly have an impact on the BlocBuilder implementation. Seems like you are thinking of bloc/cubit as a concrete implementation that you want to abstract but I think about bloc/cubit as the interface and your custom bloc/cubit class as the concrete implementation.
Actually since I declared mapEventToState like this it actually is pretty type-safe:
How does that help? If you have overridden the state
getter to downcast it is still unsafe.
Actually since I declared mapEventToState like this it actually is pretty type-safe:
How does that help? If you have overridden the state getter to downcast it is still unsafe.
Doesn't help on initial state I suppose, but unless you are violating the guideline that you should not call emit from a bloc the only way that state gets changed is the return value from mapEventToState
Hey there!
First of all @dalewking , you really need to work on your social skills. Implement on yourself the GoodManners
"interface", would you?
Secondly, before dropping "knowledge" I'd highly advise you to get a little bit more familiar with basic concepts of the dart language and flutter framework. Just cause you were used to do some stuff in a way in some other framework or language doesn't mean the same exact concept would be totally appliable in flutter.
For example there are no interfaces in dart but instead of that abstract classes are being used to provide a contract for concrete implementations.
If you knew that you'd have understood right from the get go that Cubit
and Bloc
are your abstractions and the ones created by you are the concrete implementations. You probably got confused by the class
keyword and also missed the abstract
one, happens to any beginner so nothing to be worried about. 😊
You can always use a mixin on your state to make sure you don't forget to implement something.
Yes but my intention was to restrict it to bloc/cubit since that is what it was implemented to be used with. Just because the explicit API surface area it is interacting with is smaller than the surface area exposed by bloc/cubit doesn't mean it makes sense to create another, more generic interface. For example, until recently blocs and cubits emitted the previous state upon subscription (like a BehaviorSubject) and recently this behavior was changed. While technically the API surface didn't change, the behavior did and it would certainly have an impact on the BlocBuilder implementation. Seems like you are thinking of bloc/cubit as a concrete implementation that you want to abstract but I think about bloc/cubit as the interface and your custom bloc/cubit class as the concrete implementation.
And my intention is make the UI completely agnostic as to what the underlying state mechanism is. Until recently I was a fan of redux and only recently started switching to Bloc (primarily because there was no consensus on what Blocs looked like. Is there a bloc per screen or multiple Blocs). I would really like the UI to have no dependency on whether you used redux, bloc, or just raw streams/RX subjects underneath. I have kind of recoiled at the whole notion that you have BlocBuilder in your UI if you use Bloc and StoreBuilder if you use redux. I have actually done redux and only used StreamBuilder in the UI.
The only real differences I see with StoreBuilder or BlocBuilder vs. Stream Builder is where initial state is handled. In Stream Builder the UI has to handle it, with StoreBuilder and BlocBuilder the Store or Bloc handles it. There is also of course the use of AsyncShapshot on StreamBuilder, but that is really due to the fact that initial state is optional with StreamBuilder.
What I really want to see is a Builder class that can be used with Bloc or Redux or some other scheme so the UI doesn't actually care and is not tied to one or the other. I personally don't think conceptually that there is that much different between redux and bloc. You have a current state, events go in, asynchronous operations can occur and then the state is changed and sent on a stream. There is certainly differences in the implementation but at a block diagram level they are pretty much the same. (And I am not one of those people that believed that in redux there was only one store for the entire app).
According to the redux documentation, the first core principle of redux is:
Single source of truth The global state of your application is stored in an object tree within a single store.
In any case, there's nothing preventing you from creating your own version of BlocBuilder which interops with both (although I'm not sure what the benefit is). It hasn't been very often that I've wanted to have multiple state management solutions used interchangeably within a single application but if it works for you then go for it 👍
Feels like this conversation has diverged from the original issue so is it ok to close it now?
First of all @dalewking , you really need to work on your social skills. Implement on yourself the GoodManners "interface", would you?
The only time I got a little nasty is when I raised a valid question and then topic was just abruptly closed like my question had been addressed. In a real world analogy it would be as though he just hung up on my phone call.
Secondly, before dropping "knowledge" I'd highly advise you to get a little bit more familiar with basic concepts of the dart language and flutter framework. Just cause you were used to do some stuff in a way in some other framework or language doesn't mean the same exact concept would be totally appliable in flutter.
For example there are no interfaces in dart but instead of that abstract classes are being used to provide a contract for concrete implementations.
I understand that there is no interface key word in Dart and never did I imply other wise. I only used interface in the abstract sense of the way you interact with a system. That use of the word is completely separate from abstract classes The interface in that sense could be an abstract class or could be a concrete class. I believe that until very recently BlocBuilder accepted a parameter of Bloc so the "interface" it used was a concrete class which violated good design principles. The move to Cubit was a step in the right direction, but I think it could go further so that it interacts with an abstract class with no implementation and only includes what is needed by BlocBuilder.
If you knew that you'd have understood right from the get go that Cubit and Bloc are your abstractions and the ones created by you are the concrete implementations. You probably got confused by the class keyword and also missed the abstract one, happens to any beginner so nothing to be worried about. 😊
Excuse me?!? I'm pretty sure I've been coding longer than you have been alive, probably even doing OOP programming back when you were in diapers.
And my point is that Cubit and Bloc are not good abstractions to use for BlocBuilder because they tie you to the implementation.
You can always use a mixin on your state to make sure you don't forget to implement something.
Which is a completely meaningless statement in this context.
According to the redux documentation, the first core principle of redux is:
The global state of your application is stored in an object tree within a single store.
Oh, I know some redux proponents say that and it sounds like a good idea but it is totally unworkable except for the most simple of applications. First there is the concept of local vs. global state. You may have global state like are you logged in. But you can have local state like the validation for a form view which makes no sense to maintain in some global state. And it really breaks down when you can have multiple views that kind of need their own copy of the same type of state. Take that form validation state, what if I can have multiple instances of that form open at once like in an application that has multiple windows?
I discussed it here: https://github.com/brianegan/flutter_redux/issues/109
In any case, there's nothing preventing you from creating your own version of BlocBuilder which interops with both (although I'm not sure what the benefit is). It hasn't been very often that I've wanted to have multiple state management solutions used interchangeably within a single application but if it works for you then go for it 👍
It isn't that you are going to have both in the same app (although you might if you were transitioning from one the other), but designing the UI so that it doesn't know the difference.
Feels like this conversation has diverged from the original issue so is it ok to close it now?
I think I will look into creating a unified builder that can replace both StoreBuilder/StoreConnector and BlocBuilder based around the StateStream abstract class I posted earlier. It is trivial to wrap a Bloc or a Store into an implementation of that interface. I think what I might do is investigate is combining
Excuse me?!? I'm pretty sure I've been coding longer than you have been alive, probably even doing OOP programming back when you were in diapers.
I've seen your avatar. I'm sure you were coding and I was in diapers. I'm also coding and you're in diapers now.
import 'package:bloc/bloc.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; abstract class FooState { bool get foo; } abstract class FooBloc<E, S extends FooState> extends Bloc<E, S> { FooBloc(S state) : super(state); } class FooStateImpl implements FooState { const FooStateImpl(this.foo, this.bar); final bool foo; final int bar; FooStateImpl copyWith({bool foo, int bar}) => FooStateImpl(foo ?? this.foo, bar ?? this.bar); } class MyBloc extends FooBloc<int, FooState> { MyBloc() : super(FooStateImpl(false, 0)); @override FooStateImpl get state => super.state as FooStateImpl; @override Stream<FooStateImpl> mapEventToState(int event) async* { yield state.copyWith(foo: false, bar: 5); } } void main() => runApp(MyApp()); class MyApp extends StatelessWidget { @override Widget build(BuildContext context) { return MaterialApp( home: BlocProvider<FooBloc>( create: (_) => MyBloc(), child: HomePage(), ), ); } } class HomePage extends StatelessWidget { @override Widget build(BuildContext context) { return BlocBuilder<FooBloc, FooState>( builder: (context, state) { return Text(state.foo.toString()); }, ); } }
Hello @felangel and community, thanks for your awesome work, it's been a great journey for me with Bloc Library. Apologies if the question is so simple, but I'd like to learn :smile: When we provide FooState
to Widget with BlocProvider
, we are not able to access the concrete state's members since the type is FooState right? If so, how can we access to concrete state's members?
My solution to this issue is cast FooState
to FooStateImpl
in bloc's builder method. Then use concrete state's members.
Came here to reply to a statement directed at me but apparently that has been removed.
I have since moved on to StateNotifier and river pod hooks (along with a dart version of MviKotlin that I am working on) so bloc isn't even on my radar any more.
StateNotifier by the way is exactly the type of abstraction I was asking for in this issue, a type that represents a value that can be retrieved and listened to. I was proposing that such an interface be extracted and some one else did the job. So you could wrap whichever state management paradigm you use in a type that extended StateNotifier and implemented whatever interface you want to use to send events to it. Then you can craft your UI using StateNotifierBuilder or the multiple ways that river pod allows and your UI doesn't care what the underlying state management is, which was my goal..
Is your feature request related to a problem? Please describe. I want to declare an interface for my Bloc state such that the view where BlocBuilder is created only accesses the interface and the implementation of that state is hidden away from it.
So imagine I have these definitions available to my view class and I can pass an instance of FooBloc to the view using injection:
Then in an another part of the system I would like to declare implementations of those:
But that does not work and you get errors that Bloc<int, FooStateImpl> and Cubit<int, FooState> are different types.
Describe the solution you'd like With all the functionality of Cubit I don't think you can make Bloc<int, FooStateImpl> be a subclass of Cubit<int, FooState>. However for BlocBuilder it doesn't need all of Cubit. I believe all that BlocBuilder needs is listen, state, and cancel.
I think there needs to be an abstract class above Cubit that describes only the things needed by BlocBuilder and supports covariance, in other words out variance as this class would only have methods that return values and never accepts values (see the article linked below on variance) and BlocBuilder be changed to use this interface above Cubit.
Describe alternatives you've considered The only workaround is to throw away data hiding and have a concrete class.
Additional context Article on declaration site variance in Dart