Open mgj opened 1 year ago
@mgj
Instead of calling init()
from the BlocBuilder, you should call it in the BlocProvider's create
method.
In your example it would look like this:
class MyApp extends StatelessWidget {
const MyApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
title: 'Flutter Demo',
theme: ThemeData(
primarySwatch: Colors.blue,
),
home: BlocProvider(
create: ((context) => CubitBasedCubit()..init()), // <--- initialization events and actions
child: const CubitBasedPage(),
),
/*
home: BlocProvider(
create: ((context) => BlocBasedCubit()),
child: const BlocBasedPage(),
),
*/
/*
home: BlocProvider(
create: ((context) => CubitBasedWorkaroundCubit()),
child: const CubitBasedWorkaroundPage(),
),
*/
);
}
}
Hej @mgj, generally speaking I would say that you should never put logic like this (non-ui) in your build
method, as you are doing here https://github.com/mgj/flutter_bloc_cubit_issue/blob/main/lib/pages/cubitbased/cubitbased_page.dart#L19
@override
Widget build(BuildContext context) {
return Scaffold(body: BlocBuilder<CubitBasedCubit, CubitBasedState>(
builder: (context, state) {
final cubit = context.read<CubitBasedCubit>();
if (cubit.state is CubitBasedInitialState) {
cubit.init(); // don't do this
return const CircularProgressIndicator();
}
// ...
},
));
Your build method should only build a widget according to its state, not communicate with blocs and other services. One reasoning behind that might be that the build
method might never be called at all, might be called once, might be called multiple times, and that will make your app unpredictable.
If you want to listen for your state changes, use listen
method in BlocListener
or BlocConsumer
. In this callback method, you can listen to cubit state changes and add events (calling cubit methods) to the bloc.
As for the calling of the init()
method, you can call it from the create
callback of BlocProvider
as @jacobtipp suggests, as it is the perfect time and place to call such an event.
Thanks @jacobtipp and @tenhobi for taking the time to respond, I appreciate it.
I understand there are workarounds for this issue but that does not mean that there is not still a problem - It should never be possible to get into a situation where the Page is not notified of the last/final emitted State.
As for using BlocListener
its not actually possible to do that for initialization since the listen
in BlocListener
is only called on state changes, not for the initial state.
Hmm, the behaviour of Cubit vs Bloc in matter of what will be logged is indeed different. But that is, I suppose, related to from which context you trigger the event. One is from a synchronous context, aka when using Cubits. Second is from asynchronous event, aka when using Bloc.
Using sinks for event (bloc) instead of synchronous function (cubit) makes a bit of "delay" which you can somehow "simulate" using scheduleMicrotask
or SchedulerBinding.instance.addPostFrameCallback
:
builder: (context, state) {
final cubit = context.read<CubitBasedCubit>();
if (cubit.state is CubitBasedInitialState) {
log("builder - initial");
scheduleMicrotask(() {
cubit.init();
});
return const CircularProgressIndicator();
}
// ...
}
Also, when you write a test, you would discover that all the event emitted are the same no matter if that is a bloc or cubit:
blocTest<CubitBasedCubit, CubitBasedState>(
'TODO: description',
build: () => CubitBasedCubit(),
act: (bloc) async {
await bloc.init();
},
expect: () => <CubitBasedState>[
CubitBasedLoadingState(),
CubitBasedReadyState(),
],
);
with output:
Expected: [Instance of 'CubitBasedLoadingState', Instance of 'CubitBasedReadyState']
Actual: [Instance of 'CubitBasedLoadingState', Instance of 'CubitBasedReadyState']
As I said, using create: ((context) => CubitBasedCubit()..init())
is not a workaround and is the right way to do the task. More thoughts:
builder: (context, state) {
if (cubit.state is CubitBasedInitialState) {
cubit.init(); // <--- don't do this
return const CircularProgressIndicator();
}
// ...
}
create
callback in Provider
is the right time and place to init the bloc, because you are creating and initialiazing the bloc there.
Because of 1, you have to work with your state, no matter if from bloc or cubit, as if you have no clue how many states and how many times each were actually rendered. Widget is a function of state. All the logic should be in your bloc.
If do not want to call init()
in create
, for some reason, you have to pass the cubit as a parameter to the stateful widget and in the initState
method, you have to schedule async event (so your widget is built before running the init()
method and therefore can listen to changes) where you call the initialisation.
@override
void initState() {
SchedulerBinding.instance.addPostFrameCallback((timeStamp) {
widget.cubit.init();
});
super.initState();
}
Thanks @jacobtipp and @tenhobi for taking the time to respond, I appreciate it.
I understand there are workarounds for this issue but that does not mean that there is not still a problem - It should never be possible to get into a situation where the Page is not notified of the last/final emitted State.
As for using
BlocListener
its not actually possible to do that for initialization since thelisten
inBlocListener
is only called on state changes, not for the initial state.
The problem would be performing side-effect code inside a build callback, that's what creates the unpredictable behavior that you are seeing. If you are familiar with react, it would be the equivalent of calling setState inside a components render method.
Calling cubit.init()
inside of a build callback is performing a side-effect.
Referring to flutter docs on the build method:
The framework calls this method when this widget is inserted into the tree in a given BuildContext and when the dependencies of this widget change (e.g., an InheritedWidget referenced by this widget changes). This method can potentially be called in every frame and should not have any side effects beyond building a widget.
Please refer to this stackoverflow answer as well:
Description I understand that some emitted states may be ignored/unhandled by the View/Page if the View is already processing/rendering a previously emitted state ( https://github.com/felangel/bloc/issues/459 ). Example:
Emit State1 Emit State2 Emit State3
Where Only State1 and State3 are actually received by the View. This is fine. However, it is possible to create a situation in which only State1 is received by the View and both State2 and State3 are lost (!). This means that the View never "catches up" to the current state.
It is possible to work around this but it seems like a very dangerous and hard-to-find pitfall. You will have tests verifying the business logic of the Cubit but then once the code is deployed, it no longer works as expected.
Steps To Reproduce
I've created a minimal example here: https://github.com/mgj/flutter_bloc_cubit_issue
The
CubitBasedPage
replicates this seemingly broken behavior, where theCubitBasedReadyState
is never reached. TheBlocBasedPage
uses Bloc instead of Cubit and does not replicate the behavior. TheCubitBasedWorkaroundPage
shows an interesting workaround where, by waiting for the first frame to be fully rendered before emitting new states, the behavior is not replicated. It is strange to me that after the first frame, this behavior is no longer replicable - almost suggesting an issue in when/how the Page subscribes to the event stream.Expected Behavior Expected to have one or more states "in the middle" of the stream be ignored, but the final/last state emitted never being ignored by the View
Screenshots No screenshots
Additional Context It seems that calling
init()
from the Cubit constructor does resolve the issue but there are 2 problems with that:Thanks in advance, please let me know if there is any additional information I can provide