Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flutter_bloc v0.20.0-rc #415

Merged
merged 1 commit into from Jul 22, 2019

Conversation

@felangel
Copy link
Owner

commented Jul 18, 2019

Status

READY

Breaking Changes

YES

Description

Automatic Bloc Lookup

Instead of having to manually do the bloc lookup for BlocBuilder and BlocListener whenever the bloc is not specified, the default behavior will be to lookup the bloc via BlocProvider automatically.

BlocBuilder Current Usage

BlocBuilder(
  bloc: BlocProvider.of<CounterBloc>(context),
  builder: (BuildContext context, int state) { ... },
) 

BlocBuilder Updated Usage

BlocBuilder<CounterBloc, int>(
  builder: (context, state) { ... },
)

BlocListener Current Usage

BlocListener(
  bloc: BlocProvider.of<CounterBloc>(context),
  listener: (int previousState, int currentState) => { ... },
)

BlocListener Updated Usage

BlocListener<CounterBloc, int>(
  listener: (previousState, currentState) => { ... },
)

BlocProvider within the same build

It is now possible to provide and access a bloc within the same build

@override
Widget build(BuildContext context) {
  return BlocProvider(
    builder: (context) => CounterBloc(),
    child: BlocBuilder<CounterBloc, int>(
        builder: (context, state) { ... }
    ),
  );
}

Todos

  • Tests
  • Documentation
  • Examples

Impact to Remaining Code Base

Breaking change which will be included in flutter_bloc v0.20.0

@felangel felangel self-assigned this Jul 18, 2019

@felangel felangel requested a review from jorgecoca Jul 18, 2019

@felangel felangel added this to In progress in bloc Jul 18, 2019

@codecov

This comment has been minimized.

Copy link

commented Jul 18, 2019

Codecov Report

Merging #415 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #415   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          11     11           
  Lines         179    198   +19     
=====================================
+ Hits          179    198   +19
Impacted Files Coverage Δ
...ages/flutter_bloc/lib/src/multi_bloc_listener.dart 100% <ø> (ø) ⬆️
packages/flutter_bloc/lib/src/bloc_builder.dart 100% <100%> (ø) ⬆️
packages/flutter_bloc/lib/src/bloc_listener.dart 100% <100%> (ø) ⬆️
packages/flutter_bloc/lib/src/bloc_provider.dart 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebdde1e...f84a696. Read the comment docs.

@axellebot

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

These small improvements that you are adding everyday are just so enjoyable for the dev experience 👍

I don't get what the breaking change is ? 🤔
Or maybe I missed understood the breaking change concept 😁

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jul 18, 2019

@axellebot thanks! I'm glad you think so 😄

Regarding the breaking change, I changed the generic templating so for developers who explicitly provided the types to BlocBuilder and BlocListener, the code would no longer compile.

Old

BlocBuilder<CounterEvent, int>(...)

New

BlocBuilder<CounterBloc, int>(...)

For developers who omit these there will be no problems but it technically is still a breaking change.

@basketball-ico

This comment has been minimized.

Copy link

commented Jul 18, 2019

@felangel I like it, I prefer the state type can be inferred with only passing the bloc type but,
seem imposible in dart. 🤔

@axellebot

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

Ho tricky ! I didn't see it ! So yeah definitely a breaking change ...
Thanks for clearing this 😊

It can be cool to inferred the state type but I guess there is no way to do so.

@felangel felangel force-pushed the automatic-bloc-lookup branch from 0c42992 to 6fe4ec7 Jul 19, 2019

@warriorCoder

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2019

Did I read it correctly that you are saying that we won't have to wrap the parent widget's build method with the BlocProvider before using the bloc in a builder with this change?

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jul 19, 2019

Did I read it correctly that you are saying that we won't have to wrap the parent widget's build method with the BlocProvider before using the bloc in a builder with this change?

yes sir 👍

@basketball-ico

This comment has been minimized.

Copy link

commented Jul 19, 2019

Deletions

The following deletions were made because they were primarily there for migration/routes.

Can you be more specific please 🤗

Regarding routes, moving forward, the rule of thumb should be: if multiple routes need access to the same bloc, the bloc must be provided above the routes.

In this way, how can I provide a bloc in scoped route? I don't like all above MaterialApp

@felangel thanks

@felangel felangel force-pushed the automatic-bloc-lookup branch 2 times, most recently from f65a06d to 6821b59 Jul 20, 2019

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jul 20, 2019

Deletions

The following deletions were made because they were primarily there for migration/routes.

Can you be more specific please 🤗

Regarding routes, moving forward, the rule of thumb should be: if multiple routes need access to the same bloc, the bloc must be provided above the routes.

In this way, how can I provide a bloc in scoped route? I don't like all above MaterialApp

@felangel thanks

I removed those deletions for now until there is a proper solution in place 👍

@felangel felangel force-pushed the automatic-bloc-lookup branch 2 times, most recently from 0f22dbc to 1c7ba5d Jul 20, 2019

@felangel felangel changed the title Add Automatic Bloc Lookup to BlocBuilder and BlocListener flutter_bloc v0.20.0 Jul 21, 2019

@felangel felangel force-pushed the automatic-bloc-lookup branch from 1c7ba5d to e909e81 Jul 21, 2019

@felangel felangel changed the title flutter_bloc v0.20.0 flutter_bloc v0.20.0-rc Jul 21, 2019

@felangel felangel force-pushed the automatic-bloc-lookup branch 3 times, most recently from 38bec3c to 7b76013 Jul 21, 2019

@anticafe

This comment has been minimized.

Copy link

commented Jul 21, 2019

About BlocProvider in build() method:

@override
Widget build(BuildContext context) {
  return BlocProvider(
    builder: (context) => CounterBloc(),
    child: BlocBuilder<CounterBloc, int>(
        builder: (context, state) { ... }
    ),
  );
}

I wonder could we make it shorter as well? Like this:

@override
Widget build(BuildContext context) {
  return BlocProvider<CounterBloc, int>(
    builder: (context, state) { ... }
  );
}
@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jul 21, 2019

About BlocProvider in build() method:

@override
Widget build(BuildContext context) {
  return BlocProvider(
    builder: (context) => CounterBloc(),
    child: BlocBuilder<CounterBloc, int>(
        builder: (context, state) { ... }
    ),
  );
}

I wonder could we make it shorter as well? Like this:

@override
Widget build(BuildContext context) {
  return BlocProvider<CounterBloc, int>(
    builder: (context, state) { ... }
  );
}

@anticafe unfortunately I don't think so because you need to actually create the bloc and if it has dependencies you need to inject them.

@amreniouinnovent

This comment has been minimized.

Copy link

commented Jul 21, 2019

@felangel
I think there should be a MultiBlockBuilder and MultiBlocListener

check my code it is not clean when I used a nested bloc builders

  BlocListener<ConfigEvent, ConfigState> _buildLoginBlocListener() {
    return BlocListener<ConfigEvent, ConfigState>(
      bloc: configBloc,
      listener: (BuildContext context, ConfigState state) {
        if (state is ConfigFailedState) {
          showSnackBar(state.configModel.message);
        }
      },
      child: BlocBuilder<ConfigEvent, ConfigState>(
        bloc: configBloc,
        builder: (BuildContext context, ConfigState configState) {
          return BlocListener<LoginEvent, LoginState>(
            bloc: loginBloc,
            listener: (BuildContext context, LoginState state) {
              if (state is LoginStoredState) {
                Navigator.of(context).pushReplacementNamed(PAGE_DASHBOARD);
              } else if (state is LoginFailedState) {
                showSnackBar(state.loginModel.message);
              }
            },
            child: BlocBuilder<LoginEvent, LoginState>(
                bloc: loginBloc,
                builder: (BuildContext context, LoginState loginState) {
                  return _buildLoginButton(
                      configState: configState, loginState: loginState);
                }),
          );
        },
      ),
    );
  }

@felangel felangel force-pushed the automatic-bloc-lookup branch from 7b76013 to f84a696 Jul 22, 2019

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jul 22, 2019

@amreniouinnovent you can currently use MultiBlocListener and you can split up your widget into multiple smaller widgets or create a new bloc that combines that relevant parts of the state from configBloc and loginBloc and then use the single bloc in BlocBuilder. Hope that helps 👍

@felangel felangel merged commit 42390c8 into master Jul 22, 2019

4 checks passed

codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to ebdde1e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@felangel felangel deleted the automatic-bloc-lookup branch Jul 22, 2019

@felangel felangel moved this from In progress to Done in bloc Jul 22, 2019

@amreniouinnovent

This comment has been minimized.

Copy link

commented Jul 23, 2019

@felangel Thank you
I added MultiBlocListener and the code is much better

MultiBlocListener(
        listeners: [
          BlocListener<ConfigBloc, ConfigState>(
            listener: (BuildContext context, ConfigState state) {
              if (state is ConfigFailedState) {
                showSnackBar(state.configModel.message);
              }
            },
          ),
          BlocListener<LoginBloc, LoginState>(
            listener: (BuildContext context, LoginState state) {
              if (state is LoginStoredState) {
                Navigator.of(context).pushReplacementNamed(PAGE_DASHBOARD);
              } else if (state is LoginFailedState) {
                showSnackBar(state.loginModel.message);
              }
            },
          ),
        ],
        child: BlocBuilder<ConfigBloc, ConfigState>(
            bloc: configBloc,
            builder: (BuildContext context, ConfigState configState) {
              return BlocBuilder<LoginBloc, LoginState>(
                  bloc: loginBloc,
                  builder: (BuildContext context, LoginState loginState) {
                    return _buildLoginButton(
                        configState: configState, loginState: loginState);
                  });
            }))
@algodave

This comment has been minimized.

Copy link

commented Jul 24, 2019

@felangel I'm upgrading from 0.15.0 and there's a lot of new good things! 😃

I was thinking to follow a 2-steps process like:

  1. Make my app compile and run
  2. Refactor based on the new patterns

I'm wondering: is Automatic Bloc Lookup "mandatory"? I have BlocBuilder usages like this:

BlocBuilder<CounterBloc, int>(
  bloc: blocObject,
  builder: (context, state) { ... },
)

where blocObject is created "the old way" (i.e. in a StatefulWidget of my app). My tests fail with:

BlocProvider.of() called with a context that does not contain a Bloc of type CounterBloc

Shall I conclude that BlocProvider must be made aware of any bloc object my app creates?

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jul 25, 2019

@algodave thanks for the positive feedback! Your 2 step approach makes perfect sense and the automatic bloc lookup is not mandatory. You should definitely be able to manually provide your own bloc instance. Are you able to share a link to a sample app which illustrates the problem? Thanks!

@algodave

This comment has been minimized.

Copy link

commented Jul 25, 2019

@felangel Thank you for your prompt reply and the good news 😉

I'll provide a sample, but I'll be able to do that on Monday.

@amreniouinnovent

This comment has been minimized.

Copy link

commented Jul 30, 2019

Nice video for this release
Flutter Bloc - AUTOMATIC LOOKUP - v0.20 (and Up), Updated Tutorial
https://www.youtube.com/watch?v=_vOpPuVfmiU

rowild added a commit to rowild/flutter-bloc-updated-tutorial that referenced this pull request Aug 7, 2019

Update code to syntax ov flutter_bloc 0.20.0
With flutter_bloc 0.20.0 the syntax changed a bit, documentated here:
felangel/bloc#415

It says it is a "breaking change", which is - at least for this application - not really true. However, since this is part of a tutorial, that is ongoing, it might be nice to keep up with the changes.  (I probably didn't get all of the places.)

Thank you, resocoder!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
7 participants
You can’t perform that action at this time.