refactor(bloc): add core interfaces

[felangel]

Status

READY

Breaking Changes

NO

Description

• refactor(bloc): add core interfaces (closes refactor: аdding interfaces #2994)

Type of Change

• ✨ New feature (non-breaking change which adds functionality)
• 🛠️ Bug fix (non-breaking change which fixes an issue)
• ❌ Breaking change (fix or feature that would cause existing functionality to change)
• 🧹 Code refactor
• ✅ Build configuration change
• 📝 Documentation
• 🗑️ Chore

[codecov]

Codecov Report

Merging #3012 (b52537f) into master (25ca1aa) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master     #3012    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            3        17    +14     
  Lines           75       362   +287     
==========================================
+ Hits            75       362   +287     

Impacted Files	Coverage Δ
packages/flutter_bloc/lib/src/bloc_builder.dart	100.00% <ø> (ø)
packages/flutter_bloc/lib/src/bloc_consumer.dart	100.00% <ø> (ø)
packages/flutter_bloc/lib/src/bloc_listener.dart	100.00% <ø> (ø)
packages/flutter_bloc/lib/src/bloc_selector.dart	100.00% <ø> (ø)
packages/bloc/lib/src/bloc.dart	100.00% <100.00%> (ø)
packages/bloc/lib/src/bloc_base.dart	100.00% <100.00%> (ø)
packages/flutter_bloc/lib/src/bloc_provider.dart	100.00% <100.00%> (ø)
packages/replay_bloc/lib/src/replay_bloc.dart
packages/replay_bloc/lib/src/replay_cubit.dart
packages/replay_bloc/lib/src/change_stack.dart
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25ca1aa...b52537f. Read the comment docs.

[beeth0ven]

Hi there!

This is my 2 cents feedback about naming. It looks like Stream has an interface called StreamView in dart:async. So we may consider names like BlocView.

And there is also a type called ValueStream in rxdart that expose getter for latest value and impelement Stream. Here are some options for our cases like StateStream, CurrentStateStream or StateStreamView. This type has getters of State get state and Stream<State> get stream.

Thanks!

[beeth0ven]

It looks like Stream has an interface called StreamView in dart:async. So we may consider names like BlocView.

After some research I found *View offten represent variant of original type like:

• UnmodifiableListView
• UnmodifiableSetView
• NonGrowableListView

So BlocView may not be a good fit for our interface name. My point here is to find a better name than Streamable, I guess there will be one which is more friendly.

Thanks.

[felangel]

It looks like Stream has an interface called StreamView in dart:async. So we may consider names like BlocView.

After some research I found *View offten represent variant of original type like:

• UnmodifiableListView
• UnmodifiableSetView
• NonGrowableListView

So BlocView may not be a good fit for our interface name. My point here is to find a better name than Streamable, I guess there will be one which is more friendly.

Thanks.

Thanks for the feedback! Why do you feel Streamable is a bad name? The reason for Streamable and StateStreamable is because they are analogous to Listenable and ValueListenable.

[beeth0ven]

Thanks for the feedback! Why do you feel Streamable is a bad name? The reason for Streamable and StateStreamable is because they are analogous to Listenable and ValueListenable.

Yeah, Streamable is not a bad name, in fact it's a good technical name. The bad feeling is directly form my heart 🙃. From my view point, names can help developer (library consumer) link the concept with previous experience, and get the bridge quickly. I've tried to search Streamable in my coding experience, but get empty response. Streamable make no sense for me. I may stand for someone that is not good at english ha.

Technical speaking. I feel Streamable may express the interface of Stream:

abstract class Streamable<T> {
  StreamSubscription<T> listen(...);
  ... // other stream method
}

When we come to these types:

abstract class Type1<T> {
  Stream<T> get stream;
}
abstract class Type2<T> {
  T get state;
}
abstract class Type3<T> {
  T get state;
  Stream<T> get stream;
}

They looks like providers/holders/getters. I'd like to list some code to get the feeling:

1. Streamable:

abstract class Streamable<T> {
  Stream<T> get stream;
}
abstract class Stateble<T> {
  T get state;
}
abstract class StateStreamable<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends StateStreamable<S>, S> .... // suppose we don't know what is `StateStreamable`,
                                                         // can we guess what it is in 5 seconds

2. Provider/Holder:

abstract class StreamProvider<T> {
  Stream<T> get stream;
}
abstract class StateProvider<T> {
  T get state;
}
abstract class StateStreamProvider<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends StateStreamProvider<S>, S> .... // suppose we don't know what is `StateStreamProvider`,
                                                             // can we guess what it is in 5 seconds

3. Getters

abstract class HasStream<T> {
  Stream<T> get stream;
}
abstract class HasState<T> {
  T get state;
}
abstract class HasStateStream<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends HasStateStream<S>, S> .... // suppose we don't know what is `HasStateStream`,
                                                        // can we guess what it is in 5 seconds

not sure if it's just me feel the later one is a bit better. I think there is a name that may help us understand the concept more easily.

Thanks!

[felangel]

Thanks for the feedback! Why do you feel Streamable is a bad name? The reason for Streamable and StateStreamable is because they are analogous to Listenable and ValueListenable.

Yeah, Streamable is not a bad name, in fact it's a good technical name. The bad feeling is directly form my heart 🙃. From my view point, names can help developer (library consumer) link the concept with previous experience, and get the bridge quickly. I've tried to search Streamable in my coding experience, but get empty response. Streamable make no sense for me. I may stand for someone that is not good at english ha.

Technical speaking. I feel Streamable may express the interface of Stream:

abstract class Streamable<T> {
  StreamSubscription<T> listen(...);
  ... // other stream method
}

When we come to these types:

abstract class Type1<T> {
  Stream<T> get stream;
}
abstract class Type2<T> {
  T get state;
}
abstract class Type3<T> {
  T get state;
  Stream<T> get stream;
}

They looks like providers/holders/getters. I'd like to list some code to get the feeling:

1. Streamable:

abstract class Streamable<T> {
  Stream<T> get stream;
}
abstract class Stateble<T> {
  T get state;
}
abstract class StateStreamable<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends StateStreamable<S>, S> .... // suppose we don't know what is `StateStreamable`,
                                                         // can we guess what it is in 5 seconds

2. Provider/Holder:

abstract class StreamProvider<T> {
  Stream<T> get stream;
}
abstract class StateProvider<T> {
  T get state;
}
abstract class StateStreamProvider<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends StateStreamProvider<S>, S> .... // suppose we don't know what is `StateStreamProvider`,
                                                             // can we guess what it is in 5 seconds

3. Getters

abstract class HasStream<T> {
  Stream<T> get stream;
}
abstract class HasState<T> {
  T get state;
}
abstract class HasStateStream<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends HasStateStream<S>, S> .... // suppose we don't know what is `HasStateStream`,
                                                        // can we guess what it is in 5 seconds

not sure if it's just me feel the later one is a bit better. I think there is a name that may help us understand the concept more easily.

Thanks!

Really appreciate the feedback and detailed response! The main issue with something like StateProvider or StateStreamProvider is it will conflict with the existing notion of Providers (from package:provider or package:riverpod).

Regarding the latter, HasStateStream, HasState, etc. while they are descriptive, I don't think they align with any other naming conventions in Dart/Flutter for similar interfaces.

To the point about being able to guess what the interface is within 5 seconds, I feel Streamable implies something that exposes a stream which can be listened to (but maybe it's just me 😅). I also think naming is really hard and ultimately the API documentation is there to make it crystal clear what the interface means.

I'd love to hear what others think/feel about the naming. I can try to put out a poll to get more feedback from the community 👍

[purplenoodlesoop]

I fully agree with @felangel that StreamProvider and HasStream are not very good. Streamable is fine, and it does go pretty well with the existing Listenable, but IMO its meaning is not quite right.

How I see it, is that Listenable is something that can be listened to, thus, Streamable is something that can be streamed, and by "something" I mean the class itself. So some class that implements the Streamable interface sounds like some entity that can be streamed itself, if that makes sense?

I see a StreamListenable as a good alternative.

[beeth0ven]

Thanks for the reply.

I have been a bit busy recently, and may not be fully awake yet. Please forgive me if my following reply have improper words 🙏.

from felangel:

Really appreciate the feedback and detailed response! The main issue with something like StateProvider or StateStreamProvider is it will conflict with the existing notion of Providers (from package:provider or package:riverpod).

I totaly agreed. This is what I am worried about if we use Providers, since developer need to tell which StreamProvider is it, they may have mix feeling.

Regarding the latter, HasStateStream, HasState, etc. while they are descriptive, I don't think they align with any other naming conventions in Dart/Flutter for similar interfaces.

Yeah, these names are not commonly used, I only found them in a swift library NSObject-Rx that introduce HasDisposeBag.

To the point about being able to guess what the interface is within 5 seconds, I feel Streamable implies something that exposes a stream which can be listened to (but maybe it's just me 😅). I also think naming is really hard and ultimately the API documentation is there to make it crystal clear what the interface means.

I'd love to hear what others think/feel about the naming. I can try to put out a poll to get more feedback from the community 👍

em, I'd love to listen to the community too ha. About the guess work (guess within 5 seconds), it is a way I used to test if the name can be widely accept. since there seem no objective way to tell if a name is good or not that good (production manager sometime use similar way to verify earily ideas with users).

from purplenoodlesoop:

How I see it, is that Listenable is something that can be listened to, thus, Streamable is something that can be streamed, and by "something" I mean the class itself. So some class that implements the Streamable interface sounds like some entity that can be streamed itself, if that makes sense?

yes, I'm expressing same thing as you.

We are all comfortable with these interface:

abstract class  Listenable {
  void listen(...);
}
abstract class Cancelable {
  void cancel();
}
abstract class Closable {
  void close();
}

If I understand this proposal correctly. what we are achieving is split object into piece of abstraction/trait:

Before:

// bellow code are not strictly correct, thay are just for concept demo
class BlocBase<State> {
  State get state => ...;
  Stream<State> get stream => ...;
  ... // other methods
}

class BlocBuilderBase<B extends BlocBase<S>, S> { // A widget that consume bloc
  // in the implemetation detail, we only need to call 2 methods from bloc:
  // bloc.state, bloc.stream
  // so this widget just need a class provide `state` and `stream`,
  // we can extract an interface for it, then reuse this widget with other bloc implementation
 ...
} 
...

After:

abstract class Streamable<S> {
  Stream<S> get stream;
}
abstract class StateStreamable<S> implements Streamable<S> {
  State get state;
}

class BlocBuilderBase<B extends StateStreamable<S>, S> { 
  // `BlocBuilderBase` can be reused if a object 
  // provide `state` and `stream` (implements StateStreamable)
 ...
} 

Maybe a issue I see is the classification of these interfaces, which category they belongs to:

abstract class Streamable<S> {
  Stream<S> get stream;
}
abstract class Stateble<T> {
  T get state;
}
abstract class StateStreamable<S> {
  State get state;
  Stream<S> get stream;
}

If they follow the *able naming convention, it's not easy to find a name for some types:

abstract class Type1 {
  User? get author;
}
abstract class Type2 {
  String? get title;
}
abstract class Type3 {
  Datetime? get creationDate;
}

Maybe it's too earily or unrealistic to discuss these. when we follow this pattern(abstraction/trait) further, we may face this naming issue. If we have a clear way to name getters from start, then this won't be a problem in the future.

I feel I'm a bit boring 😄. Ignore me if above statement make no sense to you ha.

I'm okay to follow the community. I'm fine with the community's choice. I just give feedback since there is a feedback wanted label in this PR.

Thank you.

[felangel]

I fully agree with @felangel that StreamProvider and HasStream are not very good. Streamable is fine, and it does go pretty well with the existing Listenable, but IMO its meaning is not quite right.

How I see it, is that Listenable is something that can be listened to, thus, Streamable is something that can be streamed, and by "something" I mean the class itself. So some class that implements the Streamable interface sounds like some entity that can be streamed itself, if that makes sense?

I see a StreamListenable as a good alternative.

I don't fully agree. A class that itself can be streamed is a Stream 😅

If we go with StreamListenable then my immediate thought would be that it implements Listenable which is not the case.

[beeth0ven]

Hi there!

I tried to find synonyms of provider then come with supplier:

abstract class StreamSupplier<T> {
  Stream<T> get stream;
}
abstract class StateSupplier<T> {
  T get state;
}
abstract class StateStreamSupplier<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends StateStreamSupplier<S>, S> ..

It is compatible with other types:

abstract class AuthorSupplier {
  User? get author;
}
abstract class TitleSupplier {
  String? get title;
}
abstract class CreationDateSupplier {
  Datetime? get creationDate;
}

This naming is pretty straightforward, what do you think about it 😊?

[felangel]

Hi there!

I tried to find synonyms of provider then come with supplier:

abstract class StreamSupplier<T> {
  Stream<T> get stream;
}
abstract class StateSupplier<T> {
  T get state;
}
abstract class StateStreamSupplier<T> {
  T get state;
  Stream<T> get stream;
}

// consume this name
class BlocConsumer<B extends StateStreamSupplier<S>, S> ..

It is compatible with other types:

abstract class AuthorSupplier {
  User? get author;
}
abstract class TitleSupplier {
  String? get title;
}
abstract class CreationDateSupplier {
  Datetime? get creationDate;
}

This naming is pretty straightforward, what do you think about it 😊?

Thanks for the additional feedback. I still have reservations about that naming convention because as far as I can tell Supplier is not a standard naming convention for similar interfaces (particularly not within Flutter and Dart). Whereas Streamable and StateStreamable is modeled after Listenable and ValueListenable.

[purplenoodlesoop]

EventSink is an existing interface from the dart:async library

[felangel]

Yup I went back and forth on it but ultimately decided it made more sense to have a custom EventSink so that it implements ErrorSink rather than two unrelated classes that happen to have the same API. Also if we go back to implementing EventSink from dart:async we’ll be prone to the false positives with the close_sinks lint rule.

[purplenoodlesoop]

it made more sense to have a custom EventSink

I totally agree! One thing is I am a bit worried about is the name clash :) Currently was testing this branch and had to revert to named imports in order to use both EventSinks.

Also, having two classes with the same name and relatively the same meaning from two different libraries can be confusing.

[felangel]

I totally agree! One thing is I am a bit worried about is the name clash :) Currently was testing this branch and had to revert to named imports in order to use both EventSinks.

Yes, if you want to use both you’d need named imports but is there a common use case in which you’d need to import both interfaces within the same file?

Also, having two classes with the same name and relatively the same meaning from two different libraries can be confusing.

Agreed, but I think it’d be more confusing to have Bloc implement both EventSink and ErrorSink where the two come from different libraries but have the same API.

[felangel]

@purplenoodlesoop I suppose we can rename Sink to something else like Channel, Consumer, Receiver, or Pipe 🤷‍♂️

What do you think?

[purplenoodlesoop]

Agreed, but I think it’d be more confusing to have Bloc implement both EventSink and ErrorSink where the two come from different libraries but have the same API.

100%, I agree that a custom interface is needed.

I suppose we can rename Sink to something else like Channel, Consumer, Receiver, or Pipe

What about BlocEventSink?

[felangel]

Yeah that works as well. My only concern is from reading the class name I’d assume it implements EventSink from dart:async 😅

I’m happy to make the change though I think that’s better than the other options so far 🙂

[purplenoodlesoop]

Whoops, indeed. Namings are hard 🥲

[felangel]

Yeah I know I’m terrible at naming 😅

I will update to BlocEventSink for now though since I agree that’s a better solution.

[purplenoodlesoop]

Yeah I know I’m terrible at naming 😅

No no, namings are indeed tricky :) Or we all are terrible at them lol

[beeth0ven]

Thanks for the additional feedback. I still have reservations about that naming convention because as far as I can tell Supplier is not a standard naming convention for similar interfaces (particularly not within Flutter and Dart). Whereas Streamable and StateStreamable is modeled after Listenable and ValueListenable.

Thanks for the update.

Yeah, I agreed naming is hard 😄. I may be too obsessed with naming. Sorry for that.

Since Streamable best fit the current dart naming convention, I'm fine with it now.

Thank @felangel, @purplenoodlesoop, @PlugFox , @lhmzhou for taking time to discuss and improve this library!

[felangel]

I'd like to point out that with this approach we won't be able to add events by fetching an existing instance through context, e.g.: context.read<CounterCubit>().add(...), since StateStreamableSource doesn't expose add.

This would go against the bloc pattern which relies on the user being able to add events from the presentation layer to the sink.

It should still work. The generic is still of type T where T extend StateStreamableSource but as a developer when you pass BlocProvider.of<MyBloc>(context); you’ll get an instance of MyBloc back so nothing should change from the user’s perspective.

Let me know if I missed something, thanks!

[narcodico]

I missed something, you can discard what I said.