[Proposal] Remove Event Stream from Bloc

[felangel]

Is your feature request related to a problem? Please describe.
Currently, it's possible to do something like:

bloc.event.listen((event) {
  // do something on each event
});

Describe the solution you'd like
Remove event stream since in theory it should not be necessary.

Additional context
@basketball-ico rightfully questioned what the use-case was for the feature so
@janosroden since you requested the feature in #259 can you please give your input? It would be awesome to get more context/a concrete use-case for why it is necessary. Thanks 👍

[xamelon]

For me it's usefull feature. And here my use-case:

I have TabBar widget, and other child widget can send to MainTabBar dismiss them. But what state I need to map? So, I listen event stream and dismiss TabBar widget at the right moment.

[janosroden]

Hi @felangel, sorry, I missed this thread.
So about the concrete use case, I didn't lie, I'm still use it for login/logout events :)

Usage:

userb.UserBloc.instance.event.listen((event) {
      if (event is userb.UserLoggedInBlocEvent) {
        String userId = event.loggedInState.user.userId;
        app.analytics?.event("login", parameters: {"u": userId});
      } else if (event is userb.UserLoggedOutBlocEvent) {
        String userId = event.loggedInState.user.userId;
        app.analytics?.event("logout", parameters: {"u": userId});
      }
    });

Emit (simplified):

class UserBloc extends Bloc<Event, State> {
  @override
  Stream<State> mapEventToState(Event event) async* {
    if (event is InitializeEvent) {
      // Get last state from persistence, try to login the user and if success then
      yield _recreateLoggedInState(); // Recreates the state with the proper type (LoggedInWithFacebookUserState/LoggedInWithGoogleUserState)
      dispatch(UserLoggedInBlocEvent(loggedInState: currentState, silent: true));
    }
    else if (event is LoginWithFacebookEvent) {
      yield UserLoggingInState(loginEvent: event);
      // Login the user and then
      yield LoggedInWithFacebookUserState(
                        user: _blocData.user,
                        accessToken: _blocData.facebookAccessToken);
      dispatch(UserLoggedInBlocEvent(loggedInState: currentState));
    }
    else if (event is LoginWithGoogleEvent) { /* similar to facebook */ }
    else if (event is UpdateUserProfileEvent) {
      // Do stuff, like display name change or profile picture upload
      yield _recreateLoggedInState();
      // No logged in / logged out events
    }
    else if (event is LogoutEvent) {
      State prevState = currentState;
      if (prevState is LoggedInUserState) {
              await _logoutUser();
              yield GuestUserState();

              // if UserLoggedOutBlocEvent is not an option, then hard to tell which user logged out
              dispatch(UserLoggedOutBlocEvent(loggedInState: prevState));
       }
    }
  }
}

That's my particular use case, and despite that I could imagine workarounds like a justHappened flag or prevState in the LoggedInState / GuestUserState class I think this is a cleaner way to notify other components.

Or think about a bloc which manage a collection:

bloc.event.listen((event) {
  if (event is ItemRemoved) print(event.item);
});

Alternatively you can place a lastRemovedItem in the state, or let the listener to compare the new state with the previous one.

I hope these examples are convincing enough or you have better idea for these use cases.

[basketball-ico]

@janosroden
Hello,
In which part of your code you use this?

userb.UserBloc.instance.event.listen((event) {
      if (event is userb.UserLoggedInBlocEvent) {
        String userId = event.loggedInState.user.userId;
        app.analytics?.event("login", parameters: {"u": userId});
      } else if (event is userb.UserLoggedOutBlocEvent) {
        String userId = event.loggedInState.user.userId;
        app.analytics?.event("logout", parameters: {"u": userId});
      }
    });

Anyway you can use the onEvent method in the user bloc for your analytics purpose, and also you can use bloc delegate for the same

Why you no use that?
because when you listen the event stream outside the bloc, you are using this stream like an output of the bloc, and this stream is an input, not an output, therefore you go against the bloc pattern design guidelines.

Thanks 🤗

[basketball-ico]

@xamelon
hello, can you be more specific, I can't understand your use case 🙁.
a little bit of code would help 🤗.

I think that you can have a tabBloc with a provider, and then you can change the state of the tabBar in any part of the widget tree,

Thanks. 🤗

[xamelon]

@basketball-ico
Hello! Yes, I have TabBarBloc in which help I change selected tab from other screens. But sometimes I need to dismiss tabBar, and easiest way I think is to send Dismiss event to TabBarBloc, but in which state I need map this event I don't know. So, I just listen event stream in MainTabBar widget and dismiss it if need:

this.mainTabBarBloc.event.listen((MainTabBarEvent event) {
      if (event is MainTabBarDismiss) {
          Navigator.popUntil(context, ModalRoute.withName('/'));
      }
});

Thanks 🤗

[basketball-ico]

@xamelon
hello,
In which part of your code you dispatch the MainTabBarDismiss is in a widget or in an bloc?

[janosroden]

@basketball-ico
That subscription is placed in my Application class which is a single shot class for 'stuff' like init NotificationBloc, keep location up to date etc. It's not the best design, refactor is in progress. Anyway, sure I can move it in the UserBloc, but of course this is not the only place where I want to react to such an important event like login/logout.
For example similar subscription takes place in the NotificationBloc, where we register/unregister the user in OneSignal, or in the BusinessBloc (our users have businesses which is a phisical place with location, phone etc.) where we fetch/clear businesses on login/logout.

I don't think moving all these tasks to onEvent it's a good idea or it'd solve the design problem. To trigger onEvent I still have to put events to the event stream, the only difference is that I narrowed its scope. Plus tight couple UserBloc with all of its dependencies doesn't sound good. Neither an IManageEverythingBlocDelegate.
I wouldn't use onTransition neither because it's complicated to figure out login events from state changes (see #259).

Despite all of that I understand your consern regarding the direction of the event flow. What do you think about a second stream in a mixin as described in the feature request conversation #259?
Its downside was the additional stream but it seems this is what you want. Is it?
Thanks

[xamelon]

@basketball-ico
I have SettingsScreen, where user can logout. I dispatch the MainTabBarDismiss in this widget.

void gotoSelectProduct() {
    MainTabBarBloc bloc = BlocProvider.of<MainTabBarBloc>(context);
    bloc.dispatch(MainTabBarDismiss());
  }

[basketball-ico]

@janosroden
Mmm, I understand that you don't want to make all logic in one bloc, and when you dispatch events like an LoggedIn or LoggedOut you want to different blocs do something.

The main problem with you approach is that you are dispatching events, an using this like and output of the bloc.

I see and the problem with you UserBloc is that not have state that show id the user logged in and logged out,

You can create an AuthBloc or SessionBloc, something like this

enum AuthState{ notInitialized, loggedIn, loggedOut }`

class AuthBloc extends Bloc<AuthState, AuthState>{
  @override
  AuthState get initialState => notInitialized;

  AuthState mapEventToState(state) async*{
    yield state;
  }
}

Then in your userBloc you can dispatch events to your authBloc and now, the bloc that need to do something when loggedIn or loggedOut can listen the state of authBloc.

What do you think?

[basketball-ico]

@xemelon
If you only do this when you dispatch themainTabBarDismiss event

this.mainTabBarBloc.event.listen((MainTabBarEvent event) {
      if (event is MainTabBarDismiss) {
          Navigator.popUntil(context, ModalRoute.withName('/'));
      }
});

In your SettingsScreen, you can do directly Navigator.popUntil(context, ModalRoute.withName('/'));
because this is not business logic, is only ui.

But if you do this when the user is Logged Out, you can use a blocListester that use the authBloc to do this.

[xamelon]

@basketball-ico Oh, sorry. When I wrote this, completely forgot about Navigator.popUntil(). Feeling very stupid Thanks :) вт, 4 июн. 2019 г. в 13:20, basketball-ico <notifications@github.com>:

…

@xemelon If you only do this when you dispatch themainTabBarDismiss event this.mainTabBarBloc.event.listen((MainTabBarEvent event) { if (event is MainTabBarDismiss) { Navigator.popUntil(context, ModalRoute.withName('/')); } }); In your SettingsScreen, you can do directly Navigator.popUntil(context, ModalRoute.withName('/')); because this is not business logic, is only ui. But if you do this when the user is Logged Out, you can use a blocListester that use the authBloc to do this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#326?email_source=notifications&email_token=AAS52VJTJVMQ437Q5HHN5RTPYYXT5A5CNFSM4HR6XRU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW36W6A#issuecomment-498592632>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAS52VI5RDD3SBQ6YRJHDP3PYYXT5ANCNFSM4HR6XRUQ> .

[janosroden]

@basketball-ico
I don't see how AuthBloc would solve the 'who logged out?' problem. Do you mean a LoggedOut state with lastUser property?
However it'd solve the case of LoggedIn event because there are no consecutive LoggedIn events.

And you didn't respond to the collection example. If you have a HaveItems state and a RemoveItem input event, then when the state changes you won't know which item is missing.
On the other hand, if you can emit events, then you just emit an ItemRemoved event which contain the removed item. These are two different use cases: one is listening for the current state (to visualize it for example), the other listening to how the state changed (e.g. to do something with the changed item).

Generally I think state changes are not enough in all cases, sometimes the how and why are matter.

As I writing this I'm started wondering maybe somehow put the how and why into the state could be better than the extra events... not sure.

[felangel]

@janosroden I think the how/why can either be captured in the state itself or the bloc can trigger the actions that you'd like to happen in response to receiving an event. For example, in the LoggedOut state case, you can peg whatever analytics from the bloc's onTransition (it doesn't have to happen in theBlocDelegate). I personally agree with @basketball-ico that we should try to avoid using the event stream as the output of the bloc because it violates the entire paradigm.

If you don't have any strong opposition, I am proposing to deprecate it in a patch and then fully remove it in the next minor version. Let me know and thanks for the discussion!

[felangel]

Deprecated in bloc v0.14.2 and will be removed in bloc v0.15.0.

I'm more than happy to help people refactor their code to not depend on the event stream so don't hesitate to reach out 👍

Thanks everyone!

[geopete84]

What are we supposed to use now instead of the event stream? This was very useful.

[felangel]

@geopete84 can you please describe your use case?