[Discussion] Should Cubit expose Stream API?

[felangel]

@chimon2000:

Something I immediately noticed is that because Cubit inherits from Stream, it surfaces the entire Stream API. This seems like a leaky abstraction, and I'm wondering whether there is some way that the API could be hidden?

One option I could see is for Cubit to have a protected/private CubitStream<State> stream property, rather than inherit from it. The most minimal change might be something like the following.

abstract class Cubit<State> {
  @protected
  CubitStream<State> stream;

  State get state => _stream.state;
}

@felangel:

Hi @chimon2000 👋 Thanks for opening an issue!

Can you please elaborate a bit on why you feel surfacing the Stream API is undesirable? In my opinion, this makes cubit (and bloc) a lot more versatile and powerful. Thanks 😄

@chimon2000:

I agree it makes it more powerful, but I also think the trade-off is added complexity most users may not care about. I don't think that the developer should need to know anything about Streams (which they don't) to use Cubit, however when you're intellisense presents you with the entire Stream API it might appear daunting. If the stream is a protected variable, that may be a way to convey to the user: "only use this if you really know what you're doing."

I also acknowledge that this is subjective and influenced by my initial reaction to seeing everything but state & init in my IDE. 😊

@felangel:

Thanks for clarifying! I see what you're saying and would love to hear what other members of the community think.

If most individuals would rather not have the Stream API at the surface level of Cubit we can easily move it one level lower but it would technically be a breaking change and it would not be consistent with bloc which also extends Stream. Check out https://github.com/felangel/bloc/issues/558#issuecomment-541336139 for more context around why we decided to implement Stream in the first place.

Thoughts?

@chimon2000:

Definitely should surface this one to the larger community. I guess I should have opened this issue before I suggested integrating bloc & cubit 😅

@felangel:

Haha no worries! I'll think about this some more and wait to hear what the rest of the community thinks 👍

One thing to note is you typically shouldn't need to use this within a cubit so in most cases you shouldn't bump into the Stream API unless you are looking for it. To me it doesn't feel like this is adding a lot of additional complexity but then again I'm biased 😛

@chimon2000:

Yeah, I was actually just using this for simulation purposes, I'm pretty sure i discovered this by accident while emitting a change.

@RollyPeres:

To me it looks more like a discussion about whether or not CubitStream<State> should implement StreamController<State>.

But that's not feasible because you won't be able to extend bloc from CubitStream<State> and also implement EventSink<Event>, the reason being that you cannot implement both EventSink<Event> and EventSink<State> due to difference in types.

If we'd to adhere to dart primitives design then having a stream property on Cubit<State> would imply that Cubit<State> or CubitStream<State> is a StreamController, which cannot be the case for the aforementioned reason.

@samandmoore:

I think that ideally, I'd want Cubit/Bloc to present a limited API that corresponded specifically to what makes them special when compared to a Stream. Then, I'd want there to be an asStream() method that would expose the raw stream to me if I found myself needing it. In the last 6 months of working with Bloc I've only dealt with a Bloc as a Stream once, so I'm not sure that a Bloc presenting as a Stream by default is particularly useful to me. Additionally, I've definitely had a few conversations with folks that are new to Bloc / Flutter and they've been confused by all the methods they can call on a Bloc. I suspect that with Cubit that will be even more common for newcomers because it's more method-oriented than Bloc.

I'm not sure how much it matters / how valid this is, but another potential thing to consider is that if Bloc/Cubit is a Stream, it can be plugged directly into places that work with Streams, like flutter_hooks and river_pod. Although, in that case, it's probably better to expose a custom hook / provider than to just use the Stream* ones. So, perhaps this doesn't matter that much after all haha.

I don't feel that strongly about this, but that's my 2 cents.

@RollyPeres:

@samandmoore asStream is mainly used to convert Futures to Streams. But as you already mentioned, bloc is already a Stream. But I do feel like a clean API for cubit would be beneficial(especially to newcomers) since it's method based, but having the stream getter would kinda go against the dart primitives design. Trade-offs. Tough one.🤦‍♂️

@samandmoore:

Hm. That's surprising to me that it would go against the dart primitives design. To me, it seems like easily being able to convert a value into a related but different value, e.g. a future to a stream, or in our case, a cubit/Bloc to a stream, would be a good feature of this specific primitive.

Definitely trade offs here though 🤔

[YeungKC]

I agree, in general, users don’t need to know too much. When advanced features are needed, the .stream method is also very good

[larssn]

I feel we've come full circle if we remove it again. That being said, I'm very much in favour of clean APIs.

mapEventToState gives direct access to the stream anyway, and we normally transform that, something like:

Stream<OrderState> mapEventToState(event) async* {
    if (event is CustomerNotFound) {
      yield* _showCustomerNotFound().map((error) => error.color = 'Blueish'); // <-- transform the output of function _showCustomerNotFound
    }
}

Personally, we've never used the Stream API present on Bloc. The big question is if anyone else do.

[fotiDim]

Not sure if it is a relevant point but one of the things that are unclear to me is how am I supposed to combine bloc with the FutureBuilder widget. A workaround which I came up with is to have a future as a prop in my state. However this does not seem very elegant. I suppose exposing the steam directly from bloc would help with that.

[felangel]

@larssn yeah I totally agree feels like we're going back in time haha. The idea of extending Stream and implementing Sink was to make it more natural and align with the Dart APIs which developers should be used to. We also in theory would benefit from the tooling for Streams/Sinks (close_sinks for example which is super buggy but in theory it would be nice to have). I have seen quite a few people directly map/transform bloc streams because they are comfortable with it.

@fotiDim can you give an example of when you use a FutureBuilder with a bloc rather than a StreamBuilder or BlocBuilder?

[fotiDim]

@felangel I am using bloc to manage fetching a bunch of POIs from an API and I use MapBox to draw them on a map as markers. Mapbox has an addImage() method which just loads custom assets in its engine and returns a Future. I am using a FutureGroup that waits for both addImage() and the POILoadSuccess state (with a Future prop as I wrote above) to be completed and then I am finally able to add markers to the map.

[felangel]

@fotiDim you can use cubit for this like so:

class MyCubit extends Cubit<MyState> {
  MyCubit() : super(MyState.initial());

  Future<void> loadAssets() async {
    emit(MyState.loading());
    await Future.wait([
      _future1,
      _future2,
    ]);
    emit(MyState.success());
  }
}

Then, you can use FutureBuilder if you want with myCubit.loadAssets().

[rrousselGit]

We've argued about this previously, but this discussion is even more reasons to merge state_notifier and Cubit.

With this change, both objects would be basically identical, and I do not believe that we cannot reconcile the small details around it. BlocProvider + Cubit + BlocBuilder is equivalent to StateNotifierProvider + StateNotifier + StateNotifierBuilder modulo some static variables and minor API differences.

The current situation both duplicate the effort in doc/tools, split/confuse the community, ~and hurts my feelings~. ~I've worked on this architecture since before the Google IO that mentioned Provider, and spent easily a year working on the different details (such as Freezed). So even if this was done in good faith and with a lot of work on your side too, it makes me sad that I'm competing with something so close to what I came up with.~

---

My feelings put aside, StateNotifier voluntarily does not implement Stream for both performance and simplification of the API – and instead expose a stream getter as suggested here for those who want to be fancy.

Even without mentioning auto-complete, there's no way implementing Stream will ever come close to not implementing it in terms of performances (due to streams having to deal with the event loop & co). And it also reduces the memory footprint

So I think this is a good change.

[felangel]

@rrousselGit the change for bloc (and consequently cubit) to extend Stream originated from https://github.com/felangel/bloc/issues/558 (of which you were a part).

but this discussion is even more reasons to merge state_notifier and Cubit.

Can you please clarify what you were thinking? I would love to better understand what your preferred direction would be for each library. Are you suggesting cubit is merged into state_notifier or vice-versa, how would that affect the current implementation of bloc which extends cubit as well as the remainder of the ecosystem (bloc_test, hydrated_bloc, replay_bloc, etc...)?

I've worked on this architecture since before the Google IO that mentioned Provider, and spent easily a year working on the different details (such as Freezed). So even if this was done in good faith and with a lot of work on your side too, it makes me sad that I'm competing with something so close to what I came up with.

The bloc library has been around just as long and the addition of Cubit was done purely in response to community requests and observations of how bloc was being used. I'm a bit hurt that you're insinuating I have tried to undermine your work.

In any case, I'm not here to try to prove that one approach is better than the other but rather to do what is best for the community.

[narcodico]

I've worked on this architecture since before the Google IO that mentioned Provider, and spent easily a year working on the different details (such as Freezed). So even if this was done in good faith and with a lot of work on your side too, it makes me sad that I'm competing with something so close to what I came up with.

It's funny for someone as smart as you are to act like this with any chance you get. You know there are other smart people as well in this community doing their own work! It's not all about they doing things that should satisfy you. Other people have ideas too you know ? Surely you must realize you're not the only one having good ideas, right ?

It's completely unfair and childish for you to keep trying to force yourself and your own work upon everyone with this "I'm a victim" attitudine you embrace.

You're the one taking everything as a competition I feel, everyone else is having fun building cool stuff for community.

[rrousselGit]

the change for bloc (and consequently cubit) to extend Stream originated from #558 (of which you were a part).

Bloc implementing Stream and Cubit implementing it are two different things. Bloc is an abstraction over stream chaining. Streams are part of the API because we want to use async* in mapEventToState

Cubit is a different matter. It's a wrapper over a mutable observable state. The Stream interface is implemented merely as a convenience.

The bloc library has been around just as long and the addition of Cubit was done purely in response to community requests and observations of how bloc was being used. I'm a bit hurt that you're insinuating I have tried to undermine your work.

I am not saying that. As I said, you've worked in good faith. But it is undeniable that StateNotifier and Cubit are incredibly close. We could take any of the existing Cubit examples and replace them with StateNotifier fairly easily.

Cubit:

class Counter extends Cubit<int> {
  Counter(): super(0);

  void increment() => emit(state + 1);
  void decrement() => emit(state - 1);
}

runApp(
  MultiProvider(
    providers: [
      BlocProvider(create: (_) => Counter()),
    ],
  )
)

BlocBuilder<Counter, int>(
  builder: (context, state) {
    return Text('$state', style: textTheme.headline2);
  },
),

FloatingActionButton(
  child: const Icon(Icons.remove),
  onPressed: () => context.bloc<Counter>().decrement(),
),

StateNotifier:

class Counter extends StateNotifier<int> {
  Counter(): super(0);

  void increment() => state++;
  void decrement() => state--;
}

runApp(
  MultiProvider(
    providers: [
      StateNotifierProvider<Counter, int>(create: (_) => Counter()),
    ],
  )
)

Consumer2<Counter, int>(
  builder: (context, counter, count, child) {
    return Text('$count', style: textTheme.headline2);
  },
),

FloatingActionButton(
  child: const Icon(Icons.remove),
  onPressed: () => context.read<Counter>().decrement(),
),

The bloc library has been around just as long

The bloc library has been around for a while, but Cubit is brand new. Whereas I have been recommending people to subclass ValueNotifier instead of ChangeNotifier (which ends-up with a Cubit-like syntax) since the very beginning of provider – and started pushing this architecture as soon as I finished Freezed

It's as if tomorrow I included in provider a class with a mapEventToState method. I'm sure you would dislike it, even if it benefits the community and have some subtle API changes.

Can you please clarify what you were thinking? I would love to better understand what your preferred direction would be for each library. Are you suggesting cubit is merged into state_notifier or vice-versa, how would that affect the current implementation of bloc which extends cubit as well as the remainder of the ecosystem (bloc_test, hydrated_bloc, replay_bloc, etc...)?

We should probably discuss about this in a different issue

[rrousselGit]

It's completely unfair and childish for you to keep trying to force yourself and your own work upon everyone with this "I'm a victim" attitudine you embrace.

You're the one taking everything as a competition I feel, everyone else is having fun building cool stuff for community.

I'm not forcing Felix to do anything. I've already discussed about this privately with him before and let it slide

I can accept that the packages won't be merged, especially if it benefits the community – even if it makes me sad. But that doesn't change the fact that we are duplicating the documentation/tooling effort and that it is a topic worth discussing openly.

[felangel]

Cubit is a different matter. It's a wrapper over a mutable observable state. The Stream interface is implemented merely as a convenience.

The Stream interface isn't implemented as a convenience. It is implemented so that bloc can extend it and we can get the benefit of code reuse for packages like hydrated_bloc, bloc_test, replay_bloc, flutter_bloc, angular_bloc, etc... Also, Cubit serves a larger purpose of diversifying developer's options as they can easily start with cubit for simple state and scale up to bloc with minimal changes. In my opinion, that's not something that should be overlooked.

It's as if tomorrow I included in provider a class with a mapEventToState method. I'm sure you would dislike it, even if it benefits the community and have some subtle API changes.

I wouldn't mind if you had good reason/intentions to do so -- I really don't want this to be a competition. The bloc library exists because it makes my life easier and I thought others might benefit from it as well.

[rrousselGit]

Let's continue this discussion in a separate issue – it's my bad for talking about merging packages in this issue.

I do agree with the change proposed here, which is no-longer implementing Stream. As I mentioned, this would allow removing the dependency on the event loop, which would improve the performances overall.

[felangel]

No worries! Regarding the original topic, I have a few follow-up questions which I would love to get people's thoughts on:

1. If Cubit no longer extends Stream would you prefer to still have a convenience listen API?

cubit.listen((state) {...});

Or would you instead prefer to have to listen via the stream getter:

cubit.stream.listen((state) {...});

2. @rrousselGit can you clarify what you mean by this?

As I mentioned, this would allow removing the dependency on the event loop, which would improve the performances overall.

My understanding was @chimon2000 didn't like the idea of the Stream API being directly exposed to developers because it polluted the API surface. Even if we no longer had Cubit extend Stream, it would still need to use Streams internally to be compatible with bloc so I'm not sure I follow how we would remove the dependency on the event loop.

Thanks!

[rrousselGit]

1. @rrousselGit can you clarify what you mean by this?

As I mentioned, this would allow removing the dependency on the event loop, which would improve the performances overall.

This indirectly answers the question about:

If Cubit no longer extends Stream would you prefer to still have a convenience listen API?

If we circle back to the comparison with StateNotifier, while it has a stateNotifier.stream.listen, it also offers a stateNotifier.addListener – which in fact is the primary use-case most of the time.

This listening mechanism does not rely on the event-loop and is implemented using a simple List<void Function()>.

This both consumes less memory and is significantly faster than Stream.listen (both for a small and large amount of listeners). An interesting side-effect of this is, it allows tests to be synchronous – which makes testing more flexible than a raw emits(...)

From there, Bloc could implement Cubit

[chimon2000]

Just in case participants on this issue have forgotten that we are an extension of Flutter's code of conduct, I'll repost this here.

Respect people, their identities, their culture, and their work. Be kind. Be courteous. Be welcoming. Listen. Consider and acknowledge people's points before responding.

https://github.com/flutter/flutter/blob/master/CODE_OF_CONDUCT.md

We should not resort to name-calling when someone voices their feelings. What we should strive to do is acknowledge them and try to empathize.

[chimon2000]

As someone with no allegiance to either bloc or state_notifier/provider, I would say that

1. There are definitely similarities between the two with the addition of Cubit
2. It's fair to examine whether or not Streams is a necessary dependency of Bloc.

On that second point, I actually had thought about that originally when I suggested Cubit be merged into Bloc, but I assumed that would be an idea to iterate on. If I'm being 100% transparent I'm not a huge fan of Dart's Stream implementation, but that's another can of worms.

I like the idea of exploring this conversation in another issue - maybe a formal RFC. I greatly appreciate the hard work of @felangel and @rrousselGit, and all the contributors to these projects. I think it makes sense to explore deduplication and reduce complexity wherever possible, so there's nothing wrong with asking if it is possible.

[larssn]

So about the Stream API, I wouldn't mind access to the underlying Stream(Controller) in Cubit, since I want to be able to transform it's emissions. However I don't necessarily need Cubit to inherit from Stream to accomplish this since it muddles the simplicity of the API.

I wanna move away from the normal event-driven flow since I'm interested in less indirection (which events cause), and less boilerplate (which Blocs generate a metric ton of). I like the direct nature of method calls, plus it's easier to teach new people.

While we're at the discussion of streams: Looking at the source for Cubit, I'm starting to wonder what problem it exactly tries to solve. Why do I need it to begin with? There's almost no source code in there. It's moving towards a thin wrapper around Stream, while making tasks that are easy with Stream, harder with Cubit. I'm starting to feel I could easily accomplish the same with a simple StreamController/StreamBuilder combo.

Sorry for the critique, I do consider this one of the best documented libraries I've ever used. The barrier for entry is basically zero and everyone should be able to understand and implement the BLoC pattern using it.

But I'm torn about needing it.

[felangel]

@larssn thanks so much for the feedback!

There's almost no source code in there. It's moving towards a thin wrapper around Stream, while making tasks that are easy with Stream, harder with Cubit. I'm starting to feel I could easily accomplish the same with a simple StreamController/StreamBuilder combo.

Can you please elaborate on this? The idea behind Cubit was to remove events (and consequently mapEventToState) from the bloc implementation since many developers were already using bloc in that way (but were sort of fighting the existing APIs) while still providing a consistent API integrating with Flutter, writing tests, and more. I would love to hear more about what tasks you feel are made harder with Cubit.

In my opinion using a StreamController/StreamBuilder comes with several drawbacks including:

• needing to manually manage/close subscriptions
• handling of async snapshots and seeding initialData
• lack of hooks for state changes (onChange, onError)

As always, I really appreciate your feedback and don't be sorry! Criticism is how we improve 😄

[larssn]

@felangel You're right about snapshots and initialData; it is a bit of a pain point with the plain StreamBuilder implementation.

The thing thats harder with Cubit, as I mentioned, is transforming the outgoing stream inside the cubit class. It's something I personally require quite often.

May I suggest moving Cubit closer to the vanilla way of working with Streams, but removing the pain points you listed.

needing to manually manage/close subscriptions StreamBuilder also manages your subscriptions while it's active. There's no way around having a dispose method that closes your controllers. I suppose it can be automated in the Cubit, but it's a very minor thing, but still nice to have.

handling of async snapshots and seeding initialData I think the existing approach of the initial seeding of state is a good idea, and it should definitely stay.

lack of hooks for state changes (onChange, onError) If you have access to the underlying Stream, you don't really need any custom onChange and onError logic, since Stream already supports everything you could want. It's also easy to extend that functionality with RxDart. Are you concerned that people can't handle all those Stream operators?

In my experience, it can be problematic to make things TOO simple, as it usually means flexibility will suffer. There's usually a balance, and I think giving access to the underlying Stream is a must, but I would recommend using composition instead of inheritance.

Anyway, enough of my ramblings. Thanks for listening 🙂

[rrousselGit]

The thing thats harder with Cubit, as I mentioned, is transforming the outgoing stream inside the cubit class. It's something I personally require quite often.

What does this mean out of curiosity?

[larssn]

@rrousselGit mapping, switchmapping, flatmapping, forkjoining, deboucing; whatever operator I need basically. It means having direct control of what the Cubit outputs.

Though I suppose I can technically map, through the onChange override.

[omidraha]

@rrousselGit @felangel

BlocProvider + Cubit + BlocBuilder is equivalent to StateNotifierProvider + StateNotifier + StateNotifierBuilder modulo some static variables and minor API differences.

I left a message here too. But that seems to be the main issue here. After a few months of getting acquainted with these packages (provider, bloc and now riverpod) It is clear that there are many commonalities about solving common issues. I want to say with respect, Please do not look for competition to do the same thing. Please open a new issue and fix this in the patience and long term. Development speed and new releases will not be useful when you are doing things with the same identity in parallel, and this will ultimately be to the detriment of the community.

You are both experts. Take this opportunity to build one future and lasting package for the community. Thanks.

[fotiDim]

@fotiDim you can use cubit for this like so:

class MyCubit extends Cubit<MyState> {
  MyCubit() : super(MyState.initial());

  Future<void> loadAssets() async {
    emit(MyState.loading());
    await Future.wait([
      _future1,
      _future2,
    ]);
    emit(MyState.success());
  }
}

Then, you can use FutureBuilder if you want with myCubit.loadAssets().

@felangel I understand what you suggest however I cannot do the same thing with bloc since it complains that emit should not be used directly. In some cases I guess I could downgrade my blocs to cubits but there are cases where I need to have both events and functions that emit states.

[jifalops]

IMHO, just expose the _controller.stream so it can be transformed, etc. The listen() method doesn't do anything unique except lazily initialize _controller, it just forwards parameters. In my experience this is usually a code smell. This would accomplish the same thing an allow users much more flexibility:

Stream<State> get stream => (_controller ??= StreamController<State>.broadcast()).stream;

Instead of:

StreamSubscription<State> listen(
    void Function(State) onData, {
    Function onError,
    void Function() onDone,
    bool cancelOnError,
  }) {
    _controller ??= StreamController<State>.broadcast();
    return _controller.stream.listen(
      onData,
      onError: onError,
      onDone: onDone,
      cancelOnError: cancelOnError,
    );
  }

[felangel]

@jifalops thanks for the input. The reason for the current implementation is to make blocs and cubits extend Stream as described by https://github.com/felangel/bloc/issues/558. By making the proposed change we would either need to still maintain a listen for backward compatibility or essentially revert the change described above which would be a very large breaking change.

[Lzyct]

@fotiDim you can use cubit for this like so:

class MyCubit extends Cubit<MyState> {
  MyCubit() : super(MyState.initial());

  Future<void> loadAssets() async {
    emit(MyState.loading());
    await Future.wait([
      _future1,
      _future2,
    ]);
    emit(MyState.success());
  }
}

Then, you can use FutureBuilder if you want with myCubit.loadAssets().

emit(MyState.loading()); doesn't work when loadAssets call from initState, i don't know why. But when i'm using Bloc it's works like a charm

[felangel]

Done as part of #2234 🎉