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

Update BlocProvider to automatically dispose blocs #344

Merged
merged 1 commit into from Jun 11, 2019

Conversation

@felangel
Copy link
Owner

commented Jun 11, 2019

Status

READY

Breaking Changes

YES

Description

Update BlocProvider to handle initializing and disposing the bloc to eliminate the need to create a StatefulWidget just to manage a bloc (#84).

This PR allows us to go from:

class MyApp extends StatefulWidget {
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  MyBloc _myBloc;
  
  @override
  initState() {
    super.initState();
    _myBloc = MyBloc();
  }

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      bloc: _myBloc,
      child: MyPage(),
    );
  }

  @override
  dispose() {
    _myBloc.dispose();
    super.dispose();
  }
}

To this:

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      builder: (BuildContext context) => MyBloc(),
      child: MyPage(),
    );
  }
}

Todos

  • Tests
  • Documentation
  • Examples

Impact to Remaining Code Base

  • Breaking change which will be included in flutter_bloc v0.16.0

@felangel felangel requested a review from jorgecoca Jun 11, 2019

@felangel felangel self-assigned this Jun 11, 2019

@felangel felangel force-pushed the feature/blocprovider-auto-dispose branch from bfdec42 to 5380896 Jun 11, 2019

@felangel felangel added this to In progress in bloc Jun 11, 2019

@codecov

This comment has been minimized.

Copy link

commented Jun 11, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #344   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         164    181   +17     
=====================================
+ Hits          164    181   +17
Impacted Files Coverage Δ
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 31a804b...5380896. Read the comment docs.

@felangel felangel merged commit 8bae47a into master Jun 11, 2019

4 checks passed

codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 31a804b
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 feature/blocprovider-auto-dispose branch Jun 11, 2019

@anticafe

This comment has been minimized.

Copy link

commented Jun 12, 2019

Hi, in case I want to access Bloc via BlocProvider but don't want to dispose Bloc on @override dispose() of Widget, is it possible?

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jun 12, 2019

@anticafe can you provide your use-case? With these changes our intention is to highly recommend creating/providing a bloc using the BlocProvider.

@felangel felangel moved this from In progress to Done in bloc Jun 12, 2019

@warriorCoder

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

With this change, if I still need a stateful widget can I create my blocs in the initState like I use to, or does all bloc creation have to use this new BlocProvider format?

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jun 12, 2019

@warriorCoder with this change you will no longer need to make StatefulWidgets to init and dispose your bloc. I'm still working out a couple of edge cases but hopefully this can be released with flutter_bloc 0.16.0 within the next week.

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jun 13, 2019

@anticafe #347 should address your concerns 👍

@felangel felangel referenced this pull request Jun 13, 2019

Merged

Expose dispose method on BlocProvider #347

3 of 3 tasks complete
@anticafe

This comment has been minimized.

Copy link

commented Jun 13, 2019

Hi @felangel I don't remember the reason correctly but in my own project, I don't dispose Bloc until the MyApp.dispose() is called. It seems if dispose a Bloc then next time enter this screen again, I cannot re-initialize it.

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jun 13, 2019

@anticafe I have updated all of the docs/tutorials to use the new BlocProvider introduced in flutter_bloc v0.16.0. Let me know if you have any additional questions but you should have full control over when the blocs are disposed and no longer need to create your own StatefulWidget.

@Knupper

This comment has been minimized.

Copy link

commented Jun 13, 2019

Hello @felangel ,

i have tried to update my project to the newest flutter_bloc version 16.0. I made all changed that are required, but now my App crashes when i try to access the Blocs within a ```BlocListenerTree``.

Error Message:

 BlocProvider.of() called with a context that does not contain a Bloc of type
I/flutter (14227):         `MyBloc.

App Architecture:

MyApp extends StatefulWidget{
  State<StatefulWidget> createState() => MyAppState();
}

class MyAppState extends State<MyApp> {
 @override
  Widget build(BuildContext context) {
    return BlocProviderTree(
      blocProviders: [
        BlocProvider<BlocA>(
          builder: (BuildContext context) =>
              BlocA(new FirestoreProvider()),
          dispose: (BuildContext context, BlocA bloc) => bloc.dispose(),
        ),
        BlocProvider<BlocB>(
          builder: (BuildContext context) =>
              GroupBloc(new FirestoreProvider(), new CloudProvider()),
          dispose: (BuildContext context, BlocB bloc) => bloc.dispose(),
        ),
      ],
      child: BlocListenerTree(
        blocListeners: [
          BlocListener<BlocAEvent, BlocAState>(
            bloc: BlocProvider.of<BlocA>(context),
            listener: (BuildContext context, BlocAState state) {
             // Do sth.
            },
          ),
          BlocListener<BlocBEvent, BlocBState>(
            bloc: BlocProvider.of<BlocB>(context),
            listener: (BuildContext context, BlocBStatestate) {
                 // Do sth.
              };
            },
          ),
        ],
        child: BlocBuilder(
          bloc: BlocProvider.of<BlocB>(context),
          builder: (BuildContext context, BlocBState state) {
            return new MaterialApp(
              theme: state.theme,
              debugShowCheckedModeBanner: BuildMode().isDebugMode,
              title: 'MyApp',
              home: new MainPage(),
            );
          },
        ),
      ),
    );
  }
}

This worked fine until version 16.0, so is this not longer supported or is this one of the edge cases where are you working on?

King regards

Max

@felangel

This comment has been minimized.

Copy link
Owner Author

commented Jun 13, 2019

Hi @Knupper 👋

You need to move the BlocProvider to the parent of the widget where you will be using the bloc.
In this case you can do something like:

void main() {
  runApp(
    BlocProviderTree(
      blocProviders: [
        BlocProvider<BlocA>(
          builder: (BuildContext context) =>
              BlocA(FirestoreProvider()),
          dispose: (BuildContext context, BlocA bloc) => bloc.dispose(),
        ),
        BlocProvider<BlocB>(
          builder: (BuildContext context) =>
              GroupBloc(FirestoreProvider(), CloudProvider()),
          dispose: (BuildContext context, BlocB bloc) => bloc.dispose(),
        ),
      ],
      child: MyApp(),
    ),
  );
}

Then you can refactor MyApp to just be a StatelessWidget like:

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return BlocListenerTree(
      blocListeners: [
        BlocListener<BlocAEvent, BlocAState>(
          bloc: BlocProvider.of<BlocA>(context),
          listener: (BuildContext context, BlocAState state) {
            // Do sth.
          },
        ),
        BlocListener<BlocBEvent, BlocBState>(
          bloc: BlocProvider.of<BlocB>(context),
          listener: (BuildContext context, BlocBStatestate) {
            // Do sth.
          },
        ),
      ],
      child: BlocBuilder(
        bloc: BlocProvider.of<BlocB>(context),
        builder: (BuildContext context, BlocBState state) {
          return MaterialApp(
            theme: state.theme,
            debugShowCheckedModeBanner: BuildMode().isDebugMode,
            title: 'MyApp',
            home: MainPage(),
          );
        },
      ),
    );
  }
}

Hope that helps 👍

@Knupper

This comment has been minimized.

Copy link

commented Jun 13, 2019

Thank you for your reply @felangel . This was my workaround, but i was not sure if this is the best solution for it.

Keep going with the plugin, it helps me a lot with a good architecture. :)

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