felangel/cubit

[DISCUSSION] Should Cubit expose Stream API?

chimon2000 opened this issue · 12 comments

chimon2000 commented

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 commented

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 commented

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. 😊

image

felangel commented

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 felangel/bloc#558 (comment) for more context around why we decided to implement Stream in the first place.

Thoughts?

chimon2000 commented

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

  • 😄1
felangel commented

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 😛

  • 👍1
chimon2000 commented

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

narcodico commented

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.

  • 👍4
samandmoore commented

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.

  • 👍3
narcodico commented

@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.🤦‍♂️

  • 👍2
samandmoore commented

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.

  • 👍1
samandmoore commented

Definitely trade offs here though 🤔