Is your feature request related to a problem? Please describe.
Proposal to address #174
Describe the solution you'd like
Proposing to add an optional condition
property to BlocBuilder
which takes the previousState
and currentState
as arguments and must return a bool
which determines whether or not to rebuild.
@override
Widget build(BuildContext context) {
final TimerBloc _timerBloc = BlocProvider.of<TimerBloc>(context);
return Scaffold(
appBar: AppBar(title: Text('Flutter Timer')),
body: Column(
children: <Widget>[
BlocBuilder(
bloc: _timerBloc,
builder: (context, state) => TimerText(duration: state.duration),
),
BlocBuilder(
condition: (prevState, currState) =>
currState.runtimeType != prevState.runtimeType,
bloc: _timerBloc,
builder: (context, state) => Actions(),
),
],
),
);
}
import 'package:equatable/equatable.dart';
import 'package:meta/meta.dart';
@immutable
abstract class TimerState extends Equatable {
final int duration;
TimerState(this.duration, [List props = const []])
: super([duration]..addAll(props));
}
class Ready extends TimerState {
Ready(int duration) : super(duration);
}
class Paused extends TimerState {
Paused(int duration) : super(duration);
}
class Running extends TimerState {
Running(int duration) : super(duration);
}
class Finished extends TimerState {
Finished() : super(0);
}
In the above example, the Text
widget with the remaining time will be rebuilt every time a new state is received (every second) whereas the Actions
widget will only be rebuilt if the state runtimeType
changes (Ready
-> Running
, Running
-> Paused
, etc...).
Describe alternatives you've considered
Lots of alternatives described in #174.
Yeah. Like the rebuild on change feature. Sure. This is also an alternative to putting a where condition on a stream to filter each event. I used to do this right in my StreamBuilder.
StreamBuilder(
stream: myStream.where((transition) =>
transition.oldState != transition.newState),
builder: (BuildContext context, AsyncSnapshot snapshot) {
return Container();
},
)
To build the where into the backend of the BlocBuilder widget and drop any conditions into it with a default function return true. I think that would be the simplest solution. I think the rebuildOnChange
is a better name. or shouldRebuild
I think there's a few things to consider here before making a final decision:
previousState
not being the same state rendered in the UI. I think it all depends on what point of view you are using: is the previousState
the last state that a bloc emitted, or is it the last state that was rendered on the UI? Should we track also the last rendered state (that is, the last time the condition was true)?BlocBuilders
have extra power to decide what states will make it to the UI... I think this will have an affect on logs, since you might states emitted by bloc that won't make to the UI... should this be captured in logs, or not?There could be other ways to solve this, like introducing just a BlocBuilderWithCondition
, or maybe finding a different/better way of composing blocs, so rather than sharing, you just have a second bloc doing the filtering/condition check...
I'll try to think about it more over the weekend
@jorgecoca previous state would always be last emitted. For tracking last rendered, the builder can implement onDidChangeDependencies
as for logs, it should always log all bloc outputs. The builder just affects the subscriber and offers filtering capabilities that were otherwise unavailable unless you accessed the stream directly using a StreamBuilder. I personally wouldn't hold this feature back for any issues you mentioned.
@jorgecoca thanks for the feedback here's what I'm thinking:
previousState
and currentState
should always reflect the bloc state and not the UI state. I would prefer not to include a last rendered state unless there's a compelling use-case for it.I considered a separate widget but I really like the fact that there's just a single widget used to render UI in response to state changes. I also agree that this can be accomplished using multiple blocs but it's not as intuitive and easy to manage imo. Definitely think about it over the weekend 馃憤
Seems good ! 馃憣
What will be the default condition
? I guess it will be :
condition: (prevState, currState) =>
currState != prevState,
and this way we also avoid breaking changes, right ?
I guess it will be also implemented for the BlocListener
?
@felangel the suggestion sounds good to me. I think it makes sense; I just wanted to bring those points to our attention, so we do not oversee them.
I think there might be value, though, in tracking/logging when a BlocBuilder
filters a state and does not render it in the UI.
That could avoid confusion in the long run, specially in large codebases where you might have to debug part of the code that you have not written, and you might need to debug/find an issue only with access to the logs. I think this idea would fall under the category of being predictable
: in most of the cases, when a bloc
emits a state, the expectation is that the emitted state will be rendered in the UI, so in those cases where that state might not be rendered, having a flag/message in the logs might save you some time and confusion.
What do you think?
PS. And now thinking about this, and given the history of issues that some other devs have opened in the past, I wonder if this kind of extra logging should be added when the bloc does not emit a state because currentState == previousState
.
Also, in case we would want to leave this API for experimentation/trial period, we could use the experimental
annotation from the meta
package:
https://pub.dev/documentation/meta/latest/meta/experimental-constant.html
@jorgecoca how would the bloc know? He would have to wire in a message back to it when the rebuild is ignored. If it's anything like the principle of simply notify listeners and let them do what they want with the information, where the sender is not concerned with what the reciever does with the information, then it's not the bloc's job to log that.
@axellebot the condition would default to always return true
and it would continue to work as expected when no condition is provided.
condition: (previous, current) => true
The case you're describing is already handled by the bloc itself.
PR is open (#319)
This feature is now available in flutter_bloc v0.15.0 馃帀
On my side, no matter whether the value of condition
is true
or false
, the builder
method will be called. 鈽癸笍
@Hentioe can you provide a sample app which reproduces the issue you're facing? Thanks!
@felangel same here. https://github.com/vjyanand/play/blob/master/lib/player.dart#L41
The condition
on BlocBuilder
only has an impact on rebuilds due to state changes in the bloc. There are many cases where Flutter will decide to rebuild part of the widget tree for a variety of reasons and in those cases, it does not matter if there is a condition
or not, BlocBuilder
will have no choice but to rebuild.
note, that it is called buildWhen and not condition since 5.0.0
See here
Most helpful comment
This feature is now available in flutter_bloc v0.15.0 馃帀