Bloc: State is not always sent to the BlocBuilder

[angel1st]

Describe the bug
BlocBuilder does not receive all of the states from time to time.

Steps to reproduce it
Below are the steps to reproduce it, using the sample:

1. Set a breakpoint in presentation/widgets/image_widget.dart - line 85 - to catch PlantStateImageSet state
2. Set a breakpoint in presentation/widgets/image_widget.dart - line 90 - to catch PlantStateImageIsLoading state
3. Run the app in DEBUG mode - you can make it on Android emulator as well
4. When the app loads, click on FAB button => it opens add_plant_page.dart
5. Click on camera icon - it is located on the bottom left default image corner - it will open device camera
6. Make a picture and confirm => this will return to the app and will send PlantEventNewImage event - presentation/widgets/image_widget.dart - line 24
7. The app should break at line 90 PlantStateImageIsLoading => this is the correct behaviour
8. The app breaks at line 85 PlantStateImageSet state => this is the correct behaviour

You should repeat the last 4 steps several times and combine them with updating the name as well. At some point, step 7 (break at line 90) will be omitted, which is the BUG.

Expected behavior
When I yield a specific state e.g. PlantStateImageIsLoading I expect the state to be received by the respective BlocProvider, which is not the case always.

*Logs *
Run flutter analyze and attach any output of that command below.

info - The declaration '_imageFromBase64String' isn't referenced - lib\local_data_sources\plant_table.dart:8:9 - unused_element
info - Close instances of dart.core.Sink - lib\main.dart:67:21 - close_sinks
info - Unused import: 'dart:io' - lib\presentation\blocs\plant_event.dart:2:8 - unused_import
info - Unused import: 'package:reminder/utils/global_translations.dart' - lib\presentationpages\add_plant_page.dart:5:8 - unused_import
info - Unused import: 'dart:math' - lib\presentation\widgets\add_plant_app_bar_widget.dart:1:8 - unused_import
info - Close instances of dart.core.Sink - lib\presentation\widgets\add_plant_app_bar_widget.dart:24:21 - close_sinks
info - Close instances of dart.core.Sink - lib\presentation\widgets\theme_brightness_picker_widget.dart:10:21 - close_sinks
info - The declaration '_getHeight' isn't referenced - librepositories\admob_repository.dart:70:10 - unused_element

Paste the output of running flutter doctor -v here.

[√] Flutter (Channel stable, v1.17.5, on Microsoft Windows [Version 10.0.18362.900], locale en-US)
• Flutter version 1.17.5 at d:\DevTools\flutter
• Framework revision 8af6b2f038 (9 days ago), 2020-06-30 12:53:55 -0700
• Engine revision ee76268252
• Dart version 2.8.4

[√] Android toolchain - develop for Android devices (Android SDK version 29.0.3)
• Android SDK at D:\DevTools\Android\Sdk
• Platform android-29, build-tools 29.0.3
• ANDROID_SDK_ROOT = D:\DevTools\Android\Sdk
• Java binary at: D:\DevTools\android-studio\jre\binjava
• Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)
• All Android licenses accepted.

[√] Android Studio (version 4.0)
• Android Studio at D:\DevTools\android-studio
• Flutter plugin version 47.1.2
• Dart plugin version 193.7361
• Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)

[!] Connected device
! No devices available

! Doctor found issues in 1 category.

Additional context
Have a look at the gist and more specifically to lines 37 and 55 from plant_bloc.dart file - as you can see, the only workaround which somehow fixes the issue is if I add await Future.delayed(Duration(milliseconds: 500)); - this gives some time and allows BlocBuilder to receive the state. I don't feel however this is the right way to handle the case.

[YeungKC]

Hi, @angel1st

Maybe you should add event first, then await? Because await will block.

https://gist.github.com/angel1st/986c038587273158c94b8de1eab868d4#file-image_widget-dart-L22

[angel1st]

@YeungKC - the state I yield is PlantStateImageIsLoading and I did it from plant_bloc.dart - line 36, right after I recevied PlantEventNewImage event.
I need pickedFile as a parameter of PlantEventNewImage, so I am not sure what do you mean.

[YeungKC]

Hi, @adsonpleal
This is my mistake, sorry.
could you provide your a minimal reproducible code sample?

[angel1st]

@YeungKC - while I was preparing the sample, I found the issue :-). It was that PlantStateImageIsLoading didn't differ from the previous state - the PlantStateImageIsLoading implementation was wrong. Once I fixed it to:

class PlantStateImageIsLoading extends PlantState {
  const PlantStateImageIsLoading({@required PlantsCompanion plantsCompanion})
      : super(plantsCompanion: plantsCompanion);
  @override
  List<Object> get props => [plantsCompanion.image]; // <= the fix
}

everything works just as it should be.

[angel1st]

@YeungKC - it seems my joy was a bit premature :-(. The issue is still present. Below are the steps to reproduce it, using the sample:

1. Set a breakpoint in presentation/widgets/image_widget.dart - line 85 - to catch PlantStateImageSet state
2. Set a breakpoint in presentation/widgets/image_widget.dart - line 90 - to catch PlantStateImageIsLoading state
3. Run the app in DEBUG mode - you can make it on Android emulator as well
4. When the app loads, click on FAB button => it opens add_plant_page.dart
5. Click on camera icon - it is located on the bottom left default image corner - it will open device camera
6. Make a picture and confirm => this will return to the app and will send PlantEventNewImage event - presentation/widgets/image_widget.dart - line 24
7. The app should break at line 90 PlantStateImageIsLoading => this is the correct behaviour
8. The app breaks at line 85 PlantStateImageSet state => this is the correct behaviour

You should repeat the last 4 steps several times and combine them with updating the name as well. At some point, step 7 (break at line 90) will be omitted, which is the BUG.

[angel1st]

@felangel - any chances you to have a look at this?

[felangel]

Hi @angel1st 👋
Thanks for opening an issue and sorry for the delay! Can you please share a link to a complete sample app which I can run locally to reproduce the issue? The gist appears incomplete and it would make it much easier for me to help you if you could provide a working sample, thanks! 🙏

[YeungKC]

Hi, @angel1st

The dart event loop prevents us from simply doing large calculations. If you use Isolate to run calculations, it should not block UI refresh.

According to the example you provided, I observed in the framework buildScope method that at this stage of image processing, it will not be executed. Normally, he constantly checks whether there is an element that needs to be refreshed.

This shows that we should be careful about computing.

[angel1st]

@felangel - thanks for answering. I have added steps and a sample app yesterday during the discussion with @YeungKC. You can find them here, but it seems they are not so obvious.
Hence I updated the issue with Steps to reproduce section as well.
Let me know, shall you need any further info.

[angel1st]

@YeungKC - many thanks for the feedback. Any suggestions (backed up with an example preferably), how I can resolve the issue? Feel free to update the Github code I submitted yesteday. Thanks!

[YeungKC]

Use isolates to do CPU intensive work.

[felangel]

Hi @angel1st 👋
Sorry for the delay! I just took a look and I'm not sure I understand the issue, if multiple states are yielded back-to-back faster than the rate at which the widgets are rebuilt, then BlocBuilder will not receive the intermediate state because the states changed too quickly. This is a duplicate of https://github.com/felangel/bloc/issues/173 and you can see this clearly in the following example:

import 'package:flutter/material.dart';

import 'package:bloc/bloc.dart';
import 'package:flutter_bloc/flutter_bloc.dart';

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

class CounterApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: BlocProvider(
        create: (_) => CounterBloc(),
        child: CounterPage(),
      ),
    );
  }
}

class CounterPage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('Counter')),
      body: BlocBuilder<CounterBloc, int>(
        builder: (_, count) {
          return Center(
            child: Text('$count', style: Theme.of(context).textTheme.headline1),
          );
        },
      ),
      floatingActionButton: Column(
        crossAxisAlignment: CrossAxisAlignment.end,
        mainAxisAlignment: MainAxisAlignment.end,
        children: <Widget>[
          Padding(
            padding: const EdgeInsets.symmetric(vertical: 4.0),
            child: FloatingActionButton(
              child: const Icon(Icons.add),
              onPressed: () =>
                  context.bloc<CounterBloc>().add(CounterEvent.increment),
            ),
          ),
        ],
      ),
    );
  }
}

enum CounterEvent { increment }

class CounterBloc extends Bloc<CounterEvent, int> {
  CounterBloc() : super(0);

  @override
  Stream<int> mapEventToState(CounterEvent event) async* {
    switch (event) {
      case CounterEvent.increment:
        yield state + 1;
        yield state + 1;
        yield state + 1;
        break;
    }
  }
}

If you run it and tap the floating action button you will not see BlocBuilder rebuild with 1, then 2,then 3 because they happen faster than the widget can rebuild. Instead, you'll just see it jump to 3.

Hope that clarifies things!

Closing for now but feel free to comment with additional questions and I'm happy to continue the conversation 👍

[angel1st]

@felangel - I respectfully disagree with your conclusion.
Have a look at line 36 - this is where I yield state PlantStateImageIsLoading. Then I have a part with image processing (lines 38 - 48), and after that at line 49, I yield state PlantStateImageSet. So I am pretty sure, there is plenty of time between these two states being yielded.
I am more inclined to accept what @YeungKC suggested above - since the image processing work (lines 38 - 48) is CPU intensive, it blocs buildScope.
I would appreciate your opinion on the topic as well.
Thanks!

[angel1st]

If someone is interested - have a look at the updated code. As long as I moved the image processing to isolate (in fact I am using the simplified compute call), everything goes just well - each state is sent properly and received by the BlocBuilder.
Thanks, @YeungKC!

[felangel]

@felangel - I respectfully disagree with your conclusion.
Have a look at line 36 - this is where I yield state PlantStateImageIsLoading. Then I have a part with image processing (lines 38 - 48), and after that at line 49, I yield state PlantStateImageSet. So I am pretty sure, there is plenty of time between these two states being yielded.
I am more inclined to accept what @YeungKC suggested above - since the image processing work (lines 38 - 48) is CPU intensive, it blocs buildScope.
I would appreciate your opinion on the topic as well.
Thanks!

Ah sorry I overlooked that and didn’t have time to properly take a look due to the v6.0.0 work that is currently in progress. Thanks so much to @YeungKC for helping you out and glad everything is sorted out now 😊