Bloc: [Proposal] Integrated Stream Subscriptions

Created on 18 Sep 2019  路  12Comments  路  Source: felangel/bloc

Is your feature request related to a problem? Please describe.
Currently, if I want to have a bloc react to a Stream of data, I need to inject the Stream into the bloc, create a StreamSubscription, react to the changes, and dispose of the StreamSubscription.

Reacting to Changes in TodosRepository (Current)

Manually need to inject TodosRepository and manage StreamSubscription.

class TodosBloc extends Bloc<TodosEvent, TodosState> {
    final TodosRepository todosRepository;
    StreamSubscription _todosSubscription;

    TodosBloc(this.todosRepository);

    ...

    Stream<TodosState> _mapLoadTodosToState() async* {
      _todosSubscription?.cancel();
      _todosSubscription = _todosRepository.todos().listen(
        (todos) {
          dispatch(
            TodosUpdated(todos),
          );
        },
      );
    }

    @override
    void dispose() {
      _todosSubscription?.cancel();
      super.dispose();
    }
}

Reacting to Changes in a Different Bloc (Current)

Manually need to inject OtherBloc and manage StreamSubscription.

class MyBloc extends Bloc<MyEvent, MyState> {
  final OtherBloc otherBloc;
  StreamSubscription otherBlocSubscription;

  MyBloc(this.otherBloc) {
    otherBlocSubscription = otherBloc.state.listen((state) {
        // Dispatch events here to trigger changes in MyBloc.
    });
  }

  ...

  @override
  void dispose() {
    otherBlocSubscription.cancel();
    super.dispose();
  }
}

Describe the solution you'd like
I would like to have my bloc subscribe to a Stream and handle reacting to data without the boilerplate and without having to manually manage the subscription.

Reacting to Changes in TodosRepository (Proposal)

Subscribe to the Stream of todos from the TodosRepository.

BlocProvider(
  builder: (context) {
    final todosRepository = RepositoryProvider.of<TodosRepository>(context);
    return TodosBloc(todosRepository)..subscribe(todosRepository.todos);
  },
  child: MyChild(),
)

React to new todos within the TodosBloc via onData.

class TodosBloc extends Bloc<TodosEvent, TodosState> {
    final TodosRepository todosRepository;

    TodosBloc(this.todosRepository);

    ...

    @override
    void onData(Object data) {
        if (data is List<Todo>) {
            dispatch(TodosUpdated(data));
        }
    }

Reacting to Changes in a Different Bloc (Proposal)

Subscribe to the Stream of states from OtherBloc.

BlocProvider(
  builder: (context) {
    final otherBloc = BlocProvider.of<OtherBloc>(context);
    return MyBloc()..subscribe(otherBloc.state);
  },
  child: MyChild(),
)

React to new states within MyBloc via onData.

class MyBloc extends Bloc<MyEvent, MyState> {

  ...

  @override
  void onData(Object data) {
    if (data is OtherBlocState) {
      // Dispatch events here to trigger changes in MyBloc.
    }
  }
}
wontfix

Most helpful comment

One of the selling points -at least for me- for this bloc implementation, is its simple mental model.
currently, bloc has:

  • one input type which is Event
  • one output type which is State

adding more input types (data that will be received from onData) may create confusion on where should I process bloc inputs
and will add more complexity for new people trying to learn how bloc works

My vote is to keep bloc as is, and provide more examples in the docs if needed

All 12 comments

One of the selling points -at least for me- for this bloc implementation, is its simple mental model.
currently, bloc has:

  • one input type which is Event
  • one output type which is State

adding more input types (data that will be received from onData) may create confusion on where should I process bloc inputs
and will add more complexity for new people trying to learn how bloc works

My vote is to keep bloc as is, and provide more examples in the docs if needed

I never though of this scenario but I think this is actually good implementation, since developer don't have to inject it. Does this mean if I have more than one bloc to inject, with this proposed way, does it mean only this?

MyBloc()
  ..subscribe(otherBloc.state)
  ..subscribe(otherBloc2.state)

@AhmedMozaly thanks for the feedback! It would be awesome if you could elaborate on why you feel it would add complexity? onData would be completely optional so the change would not be a breaking change and as a developer you never have to override it if you don't want to. Also, onData would be a void function so you wouldn't be able to yield states or anything. Is there something specific that you are worried about? I agree the naming could probably be improved 馃槢. The only reason I went with onData for the proposal was because it aligns with the Stream api.

@zaralockheart thanks for the feedback! Yes that's correct; you'd be able to chain subscribes 馃憤

@felangel hello
I am still not convinced with this API, tomorrow I will analyze it well :)

I have a cases when I need to change the stream, for example when the user change the account.

In this case how can I change the stream?

...
    todosBloc.subscribe(todosRepository.todos('id_1'));
    todosBloc.subscribe(todosRepository.todos('id_2'));
    todosBloc.subscribe(todosRepository.todos('id_3'));
 ...
)

This cancel the previous subscription of the same type before to subscribe to the new stream ?

Really glad @felangel for your following up with my comment
For me, it is not about the naming or OnData function signature
It's about the fact that we will have two entry points for the bloc

As I understand so far, the purpose of this proposal is to introduce a new function "OnData" that has one responsibility, To translate data received from stream subscription(s) into calls to "dispatch" function

But what if some developer decides to do more than that?

For example: if the bloc maintains local state for some reason.
And a developer in my team did manipulate local state in OnData?
This means I need to make explicit agreement with my development team that internal state manipulation should NOT be in the onData
which can be easily forgotten/ignored

That's when development complexity begins

This is my own view, but I'm open to hear form you and other members
And would be fine if the final call was to add such feature. I'm just trying to raise some concerns

I think @AhmedMozaly is right: the whole point about bloc package is to make these strong rules about input/output thing. And now adding these "convenient common-case methods" is breaking these rules + still it doesn't exhaust possible cases, because every bloc is different and developer wants it to behave in a bit different way.

I think that is great feature, would remove some boiler plate, specially with reusing state of other blocs.
As other mentioned BLoC is in/out for events/states, if we lock down on this only we would need to have subscriptions outside of the BLoc itself - I think about extension similar to translation layer.
Those translation layer would grab any Stream and produce events to dispatch to Bloc.

BlocProvider(
  builder: (context) {
    final otherBloc = BlocProvider.of<OtherBloc>(context);
    return MyBloc()..addEvent(MyOtherBlockTranslator(otherBloc));
  },
  child: MyChild(),
)

the MyOtherBlockTranslator would do all subscription etc. - is dispose would be tracked by BLoC lifecycle.
MyOtherBlockTranslator would have MyBloc instance to dispatch events to, it would never subscribe to it states (but that is optional)

That is just rough idea to keep BLoC in/out intact and do not have any logic at onData.
Downside is you create extra events for those integrations (not real events from UI) - which could share same base class, etc.

Edit: Bloc becomes an "octopus" with those "integration" translations.

I completely agree with this feature, it helps to standardize the way external streams are managed.
and I disagree with the claim that it might add complexity, it's actually the other way around.

as for the proposal itself, I think changing the names might be a good idea, e.g.
mapExternalDataToEvent instead of onData

which explicitly states that this method should only deal with the bloc through events.

I think we also need some way to distinguish between the subscriptions, having 2 streams of int for example there will be no way to tell which int came from which subscription,
so I suggest a tagging mechanism where you pass a tag object to the subscribe method, and pass that to the onData method .

Thanks everyone for the input! After further discussion and consideration I'm closing this. I agree that we should strive to keep blocs simple and lean so for now I'll just focus on improving the documentation when it comes to dealing with subscriptions 馃憤. I think there are some improvements to the existing approaches outlined in the docs so look forward to those revisions in the next few days.

Might revisit this in the future with a different proposal as a bloc extension rather than part of the core bloc library.

Again, I really appreciate everyone's input 馃檹

Super thanks @felangel for sharing proposals with us and continue discussions about
It seems that I'm not only enjoying working with bloc, but also enjoying its development process

BTW, there is an opened issue in Dart language about adding extension methods, this would be perfect match for current proposal
https://github.com/dart-lang/language/issues/41

Thanks again

Was this page helpful?
0 / 5 - 0 ratings

Related issues

clicksocial picture clicksocial  路  3Comments

wheel1992 picture wheel1992  路  3Comments

1AlexFix1 picture 1AlexFix1  路  3Comments

krusek picture krusek  路  3Comments

shawnchan2014 picture shawnchan2014  路  3Comments