Bloc: BlocListener/BlocBuilder condition confusion

[ezamagni]

Is your feature request related to a problem? Please describe.
I'm having a hard time dealing with change #678 introduced in 2.0.1 in which the behavior of the BlocListener/BlocBuilder condition parameter was changed.

• it's a breaking change that comes in a patch release: i just had to figure why a perfectly working BlocListener suddenly stopped working
• it's quite confusing for users: why would i want a condition on prevState and state in which prevState is not the previous state of the bloc as one would easily guess, but the last state that filtered through this same condition??
• it makes me unable to express what i was easily achieving before:
  i have a bloc that emits states _Ready_, _Updating_ or _Failed_. I was using a BlocListener to catch transitions from the _Updating_ state to inform the user wether an operation failed or succeeded. It was easy:

BlocListener<MyBloc, MyBlocState>(
  condition: (prevState, state) => (prevState is UpdatingState),
  listener: (context, state) {
    if (state is FailedState) {
      // show error
    } else {
      // show success
    }
  },
  child: ...
)

now i have no way to fix this without changing my widget tree or resorting to other workarounds.

Describe the solution you'd like
If there are no valid use cases or motivation for the changes introduced in #678 i would suggest to simply revert it.
I realize this would be another breaking change.

Describe alternatives you've considered
It would be great to have the best of the two worlds without a breaking change, provided that both behaviors are effectively useful.
The user should then be able to select between the two prevState meaning with an enumeration like so:

enum BlocTransitionSourceState {  // i'm terrible at names, sorry
  bloc,  // pre-2.0.1 behavior
  lastHandled // 2.0.1+ behavior
}

BlocListener<MyBloc, MyBlocState>(
  condition: (prevState, state) => (prevState is UpdatingState),
  conditionSourceState: BlocTransitionSourceState.bloc, // default would be .lastHandled for backward compatibility
  listener: (context, state) {
    ...
  },
  child: ...
)

My concern is that this solution would be quite difficult to understand.

Another solution would be having a single condition accepting a function with three parameters: prevState, state and prevBlocState, but i'm not really fond of this solution.

[bigworld12]

I agree that sudden breaking changes are bad, and I agree to the enum solution.

[felangel]

Hi @ezamagni 👋
Thanks for opening an issue!

I'm really sorry for the inconvenience! We viewed #678 as a bug fix rather than a feature request. Are you able to share a link to a sample app which reproduces the issue you're having? I would love to take a look and determine the next steps 👍

I would prefer to see if we can work with the existing implementation but obviously if that's not possible then I'm happy to look at other options.

[ezamagni]

Hi @felangel 👋
I'm attaching a little demo app to illustrate the problem.
The pubspec.yaml strictly requires flutter_bloc version 2.0.0 and you can see the intended functioning of the app.
As soon as you upgrade to ^2.0.0 the functioning breaks.
Please take a look at the README for a quick explanation of the scenario.

[felangel]

Hi @ezamagni 👋
I took a closer look and was able to resolve the issue by refactoring main.dart to:

import 'package:bloc_condition/bloc/bloc.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: BlocProvider<SimpleBloc>(
        create: (_) => SimpleBloc(),
        child: MyHomePage(),
      ),
    );
  }
}

class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text('Example'),
      ),
      body: Center(
        child: BlocListener<SimpleBloc, SimpleState>(
          condition: (prevState, currentState) =>
              currentState is! ResourceLoadingSimpleState,
          listener: (context, state) {
            Scaffold.of(context).showSnackBar(
              SnackBar(
                content: Text(
                  state is ErroredSimpleState
                      ? state.error
                      : "Resource loaded successfully",
                ),
              ),
            );
          },
          child: BlocBuilder<SimpleBloc, SimpleState>(
            builder: (context, state) {
              if (state is ResourceReadySimpleState) {
                return Text(state.resource);
              } else if (state is ResourceLoadingSimpleState) {
                return CircularProgressIndicator();
              } else {
                return Container();
              }
            },
          ),
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: () =>
            BlocProvider.of<SimpleBloc>(context).add(SimpleLoadEvent()),
        child: Icon(Icons.add),
      ),
    );
  }
}

Let me know what you think?

[ezamagni]

Hi @felangel,
thank you very much for your answer! (it also thought me about Scaffold.of(context), now I can get rid of many GlobalKeys. I still have a lot to learn about Flutter!)

Your code surely solves the problem in this very simple example, but I don't see it as an ideal solution: I'm still a bit concerned about the weird semantic that version ^2.0.1 gives to the prevState parameter, as a matter of fact your code avoids it and instead uses a condition function that only depends on currentState.
It works, but its meaning is very different: I wanted a BlocListener to check all state transitions from LoadingState, regardless of the ending state; your listener is instead triggered on every transition to a state different than LoadingState.

Let's have a more general ultra-simple example: a bloc with state A and B.
Say that I want to check _every_ time the bloc transitions from state A to state B. My understanding is that I cannot accomplish that with the current implementation of condition:
A -> B: condition is satisfied and BlocListener triggered
B -> A: condition not satisfied, BlocListener's prevState is B, so far so good
A -> B: condition is called with prevState = B, BlocListener is _not_ triggered
I've written a small demo app do demonstrate this.

Ultimately, I think that with this change, BlocListener has lost most of his expressive power.

To sum up: the point of this issue is the semantic meaning of prevState. I'm worried about it being a little overcomplicated, decoupled from the bloc itself, and possibly useless (can you provide a use case in which this version of prevState is useful?).

Sorry for being pedantic 👋😅

[felangel]

@ezamagni thanks for bringing this up. Originally the change was made for #671 but after thinking about it some more I agree the original behavior is more expressive and intuitive. Will revert to previous behavior in v3.0.0 👍

[felangel]

Addressed in #733 and will be included in v3.0.0

[Ramesh-X]

I don't think the original behavior is more intuitive.
And I still can't understand why should a BlocBuilder has to consider only the previous state of the Bloc without event considering the state that it was rebuild earlier.

State Handling

I think the problem is in the way of handling the state of the bloc.

Some are using the state as a flag

• Example in software: Almost all examples in the documentation of this library
• Other examples: In electronics state of the circuit is represented with a binary number

If you consider this way, the previous state which the BlocBuilder was rebuild is pretty much useless. All the cases can be covered by only looking the state changes of the Bloc.

It is because, in this case, what we actually need to consider in the condition of the BlocBuilder is a state transition.

Some are using the state as a set of values

• Example in software: State in StatefulWidget, Redux state
• Other examples: State matrices in robots, machine learning models, etc.

In this case, defining all the state transitions is nearly impossible. So in the condition of the BlocBuilder we have to catch the required state by looking at the values of the state. More detailed explanation is given in here (#671).

Real World Example

Consider a shop where a customer can add orders. Once a order is placed, 2 requests are sent to the shop owner and the branch manager.

Customer's mobile app will show as Accepted if both requests are accepted. Otherwise Waiting.

Shop owner and the branch manager can accept the order anytime they want and when they accept it will be notified to the customer's app with sockets.

class State {
  bool ownerAccepted;
  bool managerAccepted;

  State(this.ownerAccepted, this.managerAccepted);
}

class Event {}
class OwnerAcceptEvent extends Event {}
class ManagerAcceptEvent extends Event {}

@override
Stream<State> mapEventToState(Event e) async* {
  if (e is OwnerAcceptEvent) {
    yield State(true, state.managerAccepted);
  } else if (e is ManagerAcceptEvent) {
    yield State(state.ownerAccepted, true);
  }
}

// BlocBuilder
return BlocBuilder<MyBloc, State>(
    condition: (pre, current) => pre.ownerAccepted != current.ownerAccepted &&
        pre.managerAccepted != current.managerAccepted,
    builder: ...
);

With the original behavior, this BlocBuilder will never rebuilt. (this is the same example I described here)

There might be a work around for this example. But when the variable are String, List, Maps, etc instead of boolean, finding a work around is nearly impossible.

I think the alternatives suggested by @ezamagni would solve these problems.