felangel / bloc

A predictable state management library that helps implement the BLoC design pattern
https://bloclibrary.dev
MIT License
11.83k stars 3.4k forks source link

BLOC is being called again and again repeatedly #2895

Closed ManoVikram closed 3 years ago

ManoVikram commented 3 years ago

Description I have a BLOC to fetch all the audio files from AWS. When I call the Bloc Event( fetchAudioBloc.add(const FetchAllAudio()) ), it is rebuilding the UI again and again. Basically, the BLOC is being called again and again. I am not sure why this is happening. I can't find a workaround for this. When I comment or remove the statement that adds the BLOC Event, this issue gets vanished. Things work fine then. I guess, there is some issue with either the bloc or the place where I am adding the bloc event.

I am happy to provide more details if needed.

GitHub Link: https://github.com/ManoVikram/Audio_App

marcossevilla commented 3 years ago

hey @ManoVikram, can you specify which part are you commenting?

I took a quick look to your code and noticed you are getting the instance of the UploadAudioBloc on the audio_upload_screen.dart file:

final UploadAudioBloc uploadAudioBloc = context.watch<UploadAudioBloc>();

I suggest you only listen to the changes and see where you call setState as that can be rebuilding the UI and recalling the bloc, you will need to extract some of that logic and handle it in your bloc so you avoid rebuilding.

Would help us a lot if you can share a video about how it's working right now to replicate the issue and further help you. 👍

Also noticed Felix opened a PR a while ago, so I suggest you check it here.

ManoVikram commented 3 years ago

No. This is a different one @marcossevilla. This is not about UploadAudioBloc. This is about FetchAudioBloc.

So, basically, right now in home_screen.dart line 62(fetchAudioBloc.add(const FetchAllAudio());) is commented. If I uncomment it, I can see in the console that the AWS is invoked again and again which I guess means the BLOC is being called again and again.

To view it more clearly, you can replace the main.dart file from that GitHub repo with the one below.

import 'package:amplify_datastore/amplify_datastore.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:amplify_flutter/amplify.dart';
import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
import 'package:amplify_api/amplify_api.dart';
import 'package:amplify_storage_s3/amplify_storage_s3.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:after_layout/after_layout.dart';
import 'package:provider/provider.dart';
import 'package:shared_preferences/shared_preferences.dart';

import './amplifyconfiguration.dart';

import './screens/onBoardingScreens/onboarding_screen.dart';
import './screens/authenticationScreens/login_screen.dart';
import './screens/authenticationScreens/signup_screen.dart';
import './screens/authenticationScreens/forgot_password_screen.dart';
import './screens/authenticationScreens/reset_new_password_screen.dart';
import './screens/authenticationScreens/reset_password_successful_screen.dart';
import './screens/confirmationCodeScreen/confirmation_code_screen.dart';
import './screens/homeScreen/home_screen.dart';
import './screens/categoryScreen/category_screen.dart';
import './screens/searchScreen/search_screen.dart';
import './screens/subscriptionScreen/subscription_screen.dart';
import './screens/accountScreen/account_screen.dart';
import './screens/editAccountDetilsScreen/edit_account_details_screen.dart';
import './screens/audioUploadScreen/audio_upload_screen.dart';
import './screens/playingAudioScreen/playing_audio_screen.dart';

import './models/bloc/userAuthentication/registerNewUser/register_new_user_bloc.dart';
import './models/bloc/userAuthentication/confirmNewUser/confirm_new_user_bloc.dart';
import './models/bloc/userAuthentication/signInUser/sign_in_user_bloc.dart';
import './models/bloc/userAuthentication/forgotPassword/forgotPasswordEmail/forgot_password_bloc.dart';
import './models/bloc/userAuthentication/forgotPassword/newPasswordReset/reset_new_password_bloc.dart';
import './models/bloc/uploadAudio/upload_audio_bloc.dart';
import './models/bloc/featchAudioThumbnailURL/fetch_audio_thumbnail_url_bloc.dart';
import './models/bloc/updateUserData/update_user_data_bloc.dart';
import './models/bloc/fetchCategories/fetch_categories_bloc.dart';
import './models/bloc/fetchAudio/fetch_audio_bloc.dart';

import './models/provider/user_data.dart';

import './models/ModelProvider.dart';

class MyApp extends StatefulWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  @override
  void initState() {
    super.initState();
    _configureAmplify();
  }

  void _configureAmplify() async {
    AmplifyAuthCognito authPlugin = AmplifyAuthCognito();
    AmplifyDataStore dataStorePlugin =
        AmplifyDataStore(modelProvider: ModelProvider.instance);
    AmplifyAPI apiPlugin = AmplifyAPI();
    AmplifyStorageS3 storagePlugin = AmplifyStorageS3();

    await Amplify.addPlugins([
      authPlugin,
      dataStorePlugin,
      apiPlugin,
      storagePlugin,
    ]);

    try {
      await Amplify.configure(amplifyconfig);
    } on AmplifyAlreadyConfiguredException {
      print(
          "Tried to reconfigure Amplify; this can occur when your app restarts on Android.");
    }
  }

  @override
  Widget build(BuildContext context) {
    SystemChrome.setPreferredOrientations([
      DeviceOrientation.portraitUp,
    ]);

    return MultiBlocProvider(
      providers: [
        BlocProvider<RegisterNewUserBloc>(
          create: (context) => RegisterNewUserBloc(),
        ),
        BlocProvider<ConfirmNewUserBloc>(
          create: (context) => ConfirmNewUserBloc(),
        ),
        BlocProvider<SignInUserBloc>(
          create: (context) => SignInUserBloc(),
        ),
        BlocProvider<ForgotPasswordBloc>(
          create: (context) => ForgotPasswordBloc(),
        ),
        BlocProvider<ResetNewPasswordBloc>(
          create: (context) => ResetNewPasswordBloc(),
        ),
        /* BlocProvider<UploadAudioBloc>(
          create: (context) => UploadAudioBloc(),
        ), */
        BlocProvider<FetchAudioThumbnailUrlBloc>(
          create: (context) => FetchAudioThumbnailUrlBloc(),
        ),
        BlocProvider<UpdateUserDataBloc>(
          create: (context) => UpdateUserDataBloc(),
        ),
        BlocProvider<FetchCategoriesBloc>(
          create: (context) => FetchCategoriesBloc(),
        ),
        BlocProvider<FetchAudioBloc>(
          create: (context) => FetchAudioBloc(),
        ),
      ],
      child: MultiProvider(
        providers: [
          ChangeNotifierProvider(
            create: (context) => CurrentUserData(),
          ),
        ],
        child: MaterialApp(
          title: "Audio Entertainment",
          debugShowCheckedModeBanner: false,
          theme: ThemeData.light().copyWith(
            visualDensity: VisualDensity.adaptivePlatformDensity,
          ),
          routes: {
            LoginScreen.routeName: (context) => const LoginScreen(),
            SignupScreen.routeName: (context) => const SignupScreen(),
            ForgotPasswordScreen.routeName: (context) =>
                const ForgotPasswordScreen(),
            ResetNewPasswordScreen.routeName: (context) =>
                const ResetNewPasswordScreen(),
            ResetPasswordSuccessfulScreen.routeName: (context) =>
                const ResetPasswordSuccessfulScreen(),
            ConfirmationCodeScreen.routeName: (context) =>
                const ConfirmationCodeScreen(),
            HomeScreen.routeName: (context) => const HomeScreen(),
            /* HomeScreen.routeName: (context) => BlocProvider<FetchAudioBloc>(
                  create: (context) => FetchAudioBloc(),
                  child: const HomeScreen(),
                ), */
            CategoryScreen.routeName: (context) => const CategoryScreen(),
            SearchScreen.routeName: (context) => SearchScreen(),
            SubscriptionScreen.routeName: (context) =>
                const SubscriptionScreen(),
            AccountScreen.routeName: (context) => const AccountScreen(),
            EditAccountDetilsScreen.routeName: (context) =>
                const EditAccountDetilsScreen(),
            // AudioUploadScreen.routeName: (context) => const AudioUploadScreen(),
            AudioUploadScreen.routeName: (context) =>
                BlocProvider<UploadAudioBloc>(
                  create: (context) => UploadAudioBloc(),
                  child: const AudioUploadScreen(),
                ),
            PlayingAudioScreen.routeName: (context) =>
                const PlayingAudioScreen(),
          },
          /* home: BlocProvider(
            create: (context) => FetchAudioBloc(),
            child: const AudioApp(),
          ), */
          home: const AudioApp(),
        ),
      ),
    );
  }
}

class AudioApp extends StatefulWidget {
  const AudioApp({
    Key? key,
  }) : super(key: key);

  @override
  State<AudioApp> createState() => _AudioAppState();
}

class _AudioAppState extends State<AudioApp> with AfterLayoutMixin<AudioApp> {
  SharedPreferences? _prefs;
  bool _seen = false;

  Future<void> checkAlreadySeen() async {
    _prefs = await SharedPreferences.getInstance();
    _seen = (_prefs?.getBool("seen") ?? false);
  }

  Future<bool> _fetchSession() async {
    // final currentUserDataProvider = context.watch<CurrentUserData>();
    try {
      AuthSession res = await Amplify.Auth.fetchAuthSession(
        options: CognitoSessionOptions(getAWSCredentials: true),
      );

      /* AuthUser currentUser = await Amplify.Auth.getCurrentUser();
      User currentUserData = (await Amplify.DataStore.query(User.classType,
          where: User.EMAIL.eq(currentUser.username)))[0];

      if (currentUserData.profilePictureKey != null) {
        GetUrlResult profilePictureURL = await Amplify.Storage.getUrl(
            key: currentUserData.profilePictureKey!);

        currentUserDataProvider.setCurrentUserData = CurrentUser(
          email: currentUser.username,
          userID: currentUser.userId,
          profilePictureURL: profilePictureURL.url,
          isCreator: currentUserData.isCreator,
          name: currentUserData.name,
          description: currentUserData.description,
          followers: currentUserData.followers ?? [],
        );
      } else {
        currentUserDataProvider.setCurrentUserData = CurrentUser(
          email: currentUser.username,
          userID: currentUser.userId,
          isCreator: currentUserData.isCreator,
          name: currentUserData.name,
          description: currentUserData.description,
          followers: currentUserData.followers ?? [],
        );
      }

      print(currentUserDataProvider.currentUser.isCreator); */

      return res.isSignedIn;
    } on AuthException catch (error) {
      print(error.message);
      return false;
    }
  }

  @override
  void afterFirstLayout(BuildContext context) => checkAlreadySeen();

  @override
  Widget build(BuildContext context) {
    final FetchAudioBloc fetchAudioBloc = context.watch<FetchAudioBloc>();

    return Scaffold(
      body: FutureBuilder(
        future: checkAlreadySeen(),
        builder: (context, snapshot) {
          if (snapshot.connectionState == ConnectionState.waiting) {
            return const Center(
              child: CircularProgressIndicator(),
            );
          } else if (snapshot.connectionState == ConnectionState.done) {
            if (_seen) {
              return FutureBuilder(
                future: _fetchSession(),
                builder: (context, sessionSnapshot) {
                  if (sessionSnapshot.connectionState ==
                      ConnectionState.waiting) {
                    return const Center(
                      child: CircularProgressIndicator(),
                    );
                  } else if (sessionSnapshot.connectionState ==
                      ConnectionState.done) {
                    if (sessionSnapshot.hasData &&
                        sessionSnapshot.data == true) {
                      fetchAudioBloc.add(const FetchAllAudio());
                      return const HomeScreen();
                      /* return BlocProvider<FetchAudioBloc>(
                        create: (context) => FetchAudioBloc(),
                        child: const HomeScreen(),
                      ); */
                    } else if (sessionSnapshot.hasData &&
                        sessionSnapshot.data == false) {
                      return const LoginScreen();
                    } else {
                      return const Center(
                        child: Text("ERROR!"),
                      );
                    }
                  } else {
                    return const Center(
                      child: Text("ERROR!"),
                    );
                  }
                },
              );
            } else {
              _prefs?.setBool("seen", true);
              return const OnboardingScreen();
            }
          } else {
            return const Center(
              child: Text("ERROR!!"),
            );
          }
        },
      ),
    );
  }
}

void main() {
  runApp(const MyApp());
}

If you replace it, you can see the UI being rebuilt again and again repeatedly. This will stop if you remove or comment out the fetchAudioBloc.add(const FetchAllAudio()); in be main.dart I mentioned above. I guess the line number is 267 in the above code.

I don't know why this is happening.

Gene-Dana commented 3 years ago

Hey there ! So from the very quick skim I did, I can see that you are using nested Futurebuilders to run this function, and that's a big red flag.

I can imagine a loop of one build triggering another build, looping back and triggering the first build again and again..

What I would suggest is to rather use bloc just like the examples have it - maintaining a status in the state - and then using that in conjunction with a bloc builder in order to handle these calls.

For more context, look at the infinite list example https://github.com/felangel/bloc/blob/master/examples/flutter_infinite_list/lib/posts/view/posts_list.dart

This is not a bug in bloc nor is it actually bloc related ! Although, I think if you follow these patterns, you will be able to accomplish what you're intending to do.

ManoVikram commented 3 years ago

So, I have to create a separate bloc for that _fetchSession() function. And use a BlocBuilder to manage it to display the appropriate screen.

Basically, I have to create a new bloc and remove the inner FutureBuilder and replace it with BlocBuilder.

Am I right?

ManoVikram commented 3 years ago

Also, @Gene-Dana, I inserted print("Hello!!!") inside that inner FututerBuilder. Things are fine. Hello!!! is getting printed only once. But, when I insert the fetchAudioBloc.add(const FetchAllAudio()); statement, the screen gets updated again and again. When fetchAudioBloc.add(const FetchAllAudio()); is not done, things are working fine. Why?

Gene-Dana commented 3 years ago

In this case, I'd recommend just a new piece of state in the existing bloc state as opposed to a whole new bloc.

https://github.com/felangel/bloc/blob/a9b8a340d2440c84737bd03405bc69866321d4ee/examples/flutter_infinite_list/lib/posts/view/posts_list.dart#L20-L46

See how the status is driving the UI? By default, a CircularProgressIndicator() spins until those these new states are emitted. No business logic here (Such as futures and async functions), just pure reactions to managed state

The status is held within the PostState, and there are only three options. https://github.com/felangel/bloc/blob/a9b8a340d2440c84737bd03405bc69866321d4ee/examples/flutter_infinite_list/lib/posts/bloc/post_state.dart#L3-L14

The big point of BLoC is to separate your business logic from your UI (such as builders). That's why the PostBloc handles all the async function and updates the state that the UI reacts to. Clean separation of responsibilities

https://github.com/felangel/bloc/blob/a9b8a340d2440c84737bd03405bc69866321d4ee/examples/flutter_infinite_list/lib/posts/bloc/post_bloc.dart#L33-L60

The recommendation here: drive the UI with a status held in your bloc state. Keep all asyncs and futures to your bloc. Call events from the ui. These events map to the asyncs and futures (in the bloc) which eventually update the state. And then the UI updates accordingly.

Since you are initially calling the FetchAllAudio, on the creation of the bloc (where it's provided) you can do something like this:

https://github.com/felangel/bloc/blob/a9b8a340d2440c84737bd03405bc69866321d4ee/examples/flutter_infinite_list/lib/posts/view/posts_list.dart#L20-L46

See ...add(PostFetched()?

ManoVikram commented 3 years ago

Sorry, I can't understand 'Since you are initially calling the FetchAllAudio, on the creation of the bloc (where it's provided) you can do something like this:' this part you mentioned. Everything else you mentioned is how a bloc works. I did that.

The outer FutureBuilder just checks whether the user is coming to the app for the first time or not, that's it. The inner FutureBuilder is to check whether the user has signed in or not.

These 2 FutureBuilders work with completely different data.

As you mentioned earlier, I made a separate bloc to handle both operations. But, the issue is, the bloc will be called only when a bloc event is triggered, right? Here all these are happening in the main.dart file. So, I guess I can't call the block event even in the initState().

Am I missing something?

ManoVikram commented 3 years ago

Also, when I called the bloc event after removing the outer FutureBuilder, the problem still exists. The UI is getting rebuilt again and again. This happens only when I made a call to the bloc event, otherwise, it is good.

nathanielxd commented 3 years ago

Hello! Have you tried just calling context.read instead of context.watch?

I will honest to you, the whole app does not have the best architecture out there.

First thing I noticed is that you are still doing logic inside the UI. Second thing, you are using future builders instead of the stream-based pattern bloc offers. The whole thing is extremely hard to navigate and to debug.

ManoVikram commented 3 years ago

@nathanielxd Yes, I tried to use context.read(), it didn't work. The UI is getting rebuilt again and again. Same issue.

I accept that. By the way, do you have any GitHub repo I can refer to with good architecture? I can learn from that and it would be very helpful.

marcossevilla commented 3 years ago

@ManoVikram you can check the flutter_weather and flutter_firebase_login samples, also the new flutter_todos example can help a lot. 👍

Gene-Dana commented 3 years ago

Closing this for now ! Feel free to use this as a place to continue discussion, or join us on Discord https://discord.gg/ne7gNW2f