Emit events from blocs

[janosroden]

Hi,

I thinking about a general solution to emit events from blocs.

The concrete use case where I need it is a UserBloc where I want to emit Login/Logout events at first. UserBloc processes UpdateProfileEvents which results in a changed LoggedInUserState, so getting LoggedInUserState from the stream doesn't mean the user just logged in.
For analytics purposes I want to catch a special transition (see below)
where a guest user became a non-guest user.

This is the current implementation placed in the UserBloc. With a little work this can be refactored out of this class, but it still would be ugly.

  @override
  void onTransition(Transition<Event, State> transition) {
    super.onTransition(transition);
    State currentState = transition.currentState;

    if (currentState is GuestUserState && transition.event is LoginEvent) {
      state.skip(1).firstWhere((s) => s is! InProgressState).then((s) {
        if (s is LoggedInUserState)
          app.analytics?.event("login", parameters: {"u": s.user.userId});
      });
    }

    if (currentState is LoggedInUserState && transition.event is LogoutEvent) {
      app.analytics
          ?.event("logout", parameters: {"u": currentState.user.userId});
    }
  }

And this is what I try to achieve:

UserBloc.instance.events.listen((Event event) {
  if (event is LoggedInEvent)
    // do it
  else if (event is LoggedOutEvent)
    // do it
});

I admit that this is not so common to add it in the Bloc class so I could imagine a mixin for this:

mixin EventProviderBloc<TInEvent, TOutEvent, TState> on Bloc<TInEvent, TState> {
  // on Bloc because of dispose()

  // Stream<TOutEvent> get events => ...
  // some @protected helper like dispatch()
  // could provide implementation for onTransition/onError e.g. emit a TransitionEvent
}

Okay, emitting a TransitionEvent is tricky because of TOutEvent, but something similar would be nice. E.g. a TransitionEvent factory hook like TOutEvent _newTranstionEvent(Transition t). I know, implement a hook and demand another makes no sense, but I hope you'll have a better idea, or just leave that :)

So what do you think?

ps: one little annoying thing about the BlocDelegate: please pass the bloc as param

[felangel]

Hi @janosroden 👋
Thanks for opening this issue!

I saw your PR and based on your ask what are your thoughts on blocs having an events stream that you can listen to instead of an EventProvider?

I'm thinking the api would mimic the state api so you'd be able to do something like:

userBloc.events.listen((Event event) {
   // Do stuff
});

One thing to consider is if you override transform and filter events I'm assuming you'd want the filtered events in the events stream as well.

Let me know what you think and thanks again! 👍

[janosroden]

Adding the event stream to the Bloc class was my first tought, but I think it'd increase both instantiation time and memory consumption. This is a little bit high price for an optional feature.
Implemented as a mixin developers can decide whether they pay this price. This optional functionality also fits very well to the purpose of mixins.

I don't think the transform() is important here, these BlocEvents are responses (mostly) to the already possibly filtered Input Events, so I don't see any reason for more filtering.

Do you agree?

[felangel]

@janosroden I don't think it would increase instantiation or memory consumption because the stream already exists, it just isn't exposed publicly.

I agree about not needing additional filtering. 👍

[janosroden]

Stream already exists? I don't get how you mean. Can you elaborate?

I'm meant the extra overhead is appear if we put the new StreamController into the widely used Bloc class. People upgrades, and they get an extra controller which is unused (it's new) but still have to be initialized and disposed. In addition I think most of the existing bloc implementations don't need this feature (otherwise it would have been requested already).

Developers who want to use this feature have to change their code anyway both for class or mixin implementation.

[felangel]

Every time you dispatch an event it is added to the private event subject. Currently, the bloc is the only subscriber to the event stream but we can expose it publicly so that anyone can subscribe. Let me discuss this with the team but my initial thought is it would be relatively straightforward to expose this and there would be no breaking change or additional overhead introduced.

[janosroden]

Ah, I see the misunderstanding: if you check my PR carefully, you'll see I created a second event stream for a possibly different event hierarcy.
What you suggest is that the Event hierarcies have a common base and the bloc should simply ignore its "own" events.

Your idea is actually ... better :)
I don't see any drawbacks except a little bit wierd to call/read dispatch() instead of emitEvent()

[felangel]

Yeah I don't think we need to create additional streams.

One outstanding question I have is what's the use case for having to listen for events when you are in full control over when they are dispatched? Can't you just move whatever logic you have in response to an event before the dispatch? You can dispatch from within the bloc and expose a public wrapper around the dispatch so that from the UI you call bloc.loginPressed()

and in the bloc you have

void loginPressed() {
  if (currentState is ...) {
     // peg analytic here
  }
  dispatch(LoginPressed());
}

Or am I misunderstanding the use-case?

In addition, I agree that including the affected bloc in the onTransition in the delegate makes sense so I will discuss adding it in the next release with the team 👍

[janosroden]

It's about separation. I don't want the UserBloc has to trigger everybody who depends on it. E.g. analytics, OneSignal or clear local caches on logout. It must manage the user only. Keep it as simple as possible. More than enough to handle FB, Google, email login, profile data etc. in one class. In addition there are multi step state mappings, e.g. an InProgress state during a long operation. Do you have any concern regarding publishing the event stream, or just curious?

…

On Fri, May 3, 2019, 18:45 Felix Angelov ***@***.***> wrote: Yeah I don't think we need to create additional streams. So one outstanding question I have is what's the use case for having to listen for events when you are in full control over when they are dispatched. Can't you just move whatever logic you have in response to an event after the dispatch? You can dispatch from within the bloc and expose a public wrapper around the dispatch so that from the UI you call bloc.loginPressed() and in the bloc you have void loginPressed() { if (currentState is ...) { // peg analytic here } dispatch(LoginPressed()); } Or am I misunderstanding the use-case? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#259 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABOTMRQURQCEQ6FKVGNTLVTPTRT2XANCNFSM4HKTD3JA> .

[felangel]

@janosroden makes sense. I was just curious haha. I will try to get this as well as include the bloc in the BlocDelegate onTransition and onError by this evening. 👍

[janosroden]

Thanks! BlocDelegate won't be easy without breaking change.. Can you add onEvent because onTransition not fire without state change?

…

On Fri, May 3, 2019, 19:52 Felix Angelov ***@***.***> wrote: @janosroden <https://github.com/janosroden> makes sense. I was just curious haha. I will try to get this as well as include the bloc in the BlocDelegate onTransition and onError by this evening. 👍 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#259 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABOTMRU455OW7X2PMVOTOPDPTR3VPANCNFSM4HKTD3JA> .

[felangel]

No problem!

Yup, it will be a breaking change.

Oh so you want onEvent added to BlocDelegate?
What's a case where an event is dispatched without a state change?

[janosroden]

LoggedInEvent, because I'll yield UserLoggedInState before the event.

…

On Fri, May 3, 2019, 20:16 Felix Angelov ***@***.***> wrote: No problem! Yup, it will be a breaking change. What's a case where an event is dispatched without a state change? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#259 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABOTMRXLDAKQ6PUV7N3JQE3PTR6PFANCNFSM4HKTD3JA> .

[felangel]

Can you please explain a bit more? I'm not sure I understand. If you dispatch LoggedInEvent won't there always be a state change associated with it?

[janosroden]

No, it's a notification about that the current logged in state is the first one after a guest user state. A map method which handles LoginEvent could be If not guest state: return yield in progress yield logged in state dispatch logged in event, bloc or user object attached Please add onEvent as virtual method too. It's useful for mixins. Sorry for the style, not easy from mobile :) O

[janosroden]

Why email can't pass paragraphs? ..

…

On Fri, May 3, 2019, 20:38 Roden János ***@***.***> wrote: No, it's a notification about that the current logged in state is the first one after a guest user state. A map method which handles LoginEvent could be If not guest state: return yield in progress yield logged in state dispatch logged in event, bloc or user object attached Please add onEvent as virtual method too. It's useful for mixins. Sorry for the style, not easy from mobile :) On Fri, May 3, 2019, 20:26 Felix Angelov ***@***.***> wrote: > Can you please explain a bit more? I'm not sure I understand. If you > dispatch LoggedInEvent won't there always be a state change associated > with it? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#259 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ABOTMRV72ADSJJCG2456EP3PTR7WVANCNFSM4HKTD3JA> > . >

[janosroden]

Ok, Desktop Site helped

[felangel]

hmm okay I think I understand now. Thanks for clarifying 👍

[felangel]

Merged in #263, #267, and #268 and will be included in bloc v0.13.0 🎉

[janosroden]

Big thanks for the quick solution!