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

Question regarding best practices #1233

Closed
jaydangar opened this issue Jun 1, 2020 · 18 comments
Closed

Question regarding best practices #1233

jaydangar opened this issue Jun 1, 2020 · 18 comments
Assignees
Labels
question Further information is requested

Comments

@jaydangar
Copy link

First of all big thank you for making "our" work really smooth by creating this library, I don't know how many headaches I have been through while learning streams, sinks and controllers. Now. while learning flutter_bloc I want to ask about best practice regarding bloc provider. I have 2 approaches.

My scenario :

Suppose I have 3 pages and I want to access bloc in HomePage Widget (grandchild).

  • MainPage (parent)
    • LogInPage (child)
      -HomePage (grandchild)

First Approach :

I have created bloc for child and grandchild, lets call them LogInBloc and HomePageBloc, and to use them I am declaring them through multiBlocProvider in MainPage, and access them individually in LogInPage and HomePage class.

Second Approach :

I create provider for LogInBloc in MainPage Widget and then to access HomePageBloc I am using BlocProvider in LogInPage.

which one is the more common and best practice? are both equally suited?

@narcodico
Copy link
Contributor

Hi 👋

You should always aim at providing your bloc at the lowest level possible(the lowest common widget), so only widgets that care about it could access it.

As for your example, you should have the login bloc in your login page and home bloc in your home page, since they are dealing with 2 separate pages which don't seem to share anything between them.

@jaydangar
Copy link
Author

@RollyPeres , I have seen examples of providing a bloc required by child on the parent level, is it same as your approach? and when I need to use that bloc I use blocProvider in initState and close it in dispose method.

@narcodico
Copy link
Contributor

You provide the bloc at the common parent level so that all children who care about it could access it.
You don't have to involve initState or manually dispose it, everything is taken care of by BlocProvider if you use create function.

@jaydangar
Copy link
Author

@RollyPeres , It shows warning in my vscode, which forces me to use init and dispose method.

@narcodico
Copy link
Contributor

Please share a repo or gist which reproduces the issue your facing.

@felangel felangel added question Further information is requested waiting for response Waiting for follow up labels Jun 1, 2020
@jaydangar
Copy link
Author

jaydangar commented Jun 1, 2020

@felangel @RollyPeres I don't have a code rn, but when using the approach you suggested it always gives warning to close the sink. That's why I am using the approach in which I close the bloc manually.

@felangel
Copy link
Owner

felangel commented Jun 2, 2020

@jaydangar it's very likely that you're facing a false positive from the close_sinks lint rule. You can see the open issue tracking at dart-lang/sdk#57882. For now, you can probably safely disable the close_sinks rule in your analysis_options.yaml and just be sure that you are always creating your blocs within the BlocProvider(create: (context) {...}).

Hope that helps and closing for now but feel free to comment with additional questions and I'm happy to continue the conversation 👍

@felangel felangel closed this as completed Jun 2, 2020
@felangel felangel removed the waiting for response Waiting for follow up label Jun 2, 2020
@felangel felangel self-assigned this Jun 2, 2020
@jaydangar
Copy link
Author

jaydangar commented Jun 2, 2020

Thanks @felangel , is it okay to stick with my approach instead of relying on closing the bloc automatically? And thanks for the response.

@felangel
Copy link
Owner

felangel commented Jun 2, 2020

As long as you're creating the blocs via BlocProvider you should be good and don't need to manually dispose them. Also make sure you're providing blocs as low as possible in the widget tree so that only the widgets that need access to the bloc can access it.

@jaydangar
Copy link
Author

Thanks @felangel got my answer. Amd really appreciate your efforts.

@jaydangar
Copy link
Author

jaydangar commented Jun 19, 2020

@felangel I am still getting the following Warning. See the Image below. It seems like it's not getting disposed automatically.

Close instances of dart.core.Sink.

Screenshot 2020-06-19 at 4 17 21 PM

@jaydangar
Copy link
Author

jaydangar commented Jun 19, 2020

I am using a variable and close it in dispose method like following.

Screenshot 2020-06-19 at 4 22 26 PM

@felangel
Copy link
Owner

Hi @jaydangar 👋
No you should not dispose it manually because you are using a BlocProvider which already handles disposing it.

Check out #587 (comment) for more details.

@jaydangar
Copy link
Author

@felangel I understand, but I have a requirnment in which I need to send API call, which is through FetchCookListEvent() function. for that I need access to the bloc. So I declared global variable of that bloc and closed it in dispose method.

@felangel
Copy link
Owner

felangel commented Jun 19, 2020

You don’t need to dispose the bloc manually though. Either ignore the warning by adding // ignore: close_sinks for that line or create an analysis_options.yaml file and disable the close_sinks rule globally.

@jaydangar
Copy link
Author

jaydangar commented Jun 19, 2020

@felangel Another thing I want to mention is that, I am getting error called The getter 'userNameField' was called on null, for the following code. Can you let me know why I am getting it? I have declared the bloc as suggested. I am declaring my bloc and using it for furthrer use, inside build method.

import 'package:CookingApp/blocs/login_form_bloc.dart';
import 'package:CookingApp/utils/routing.dart';
import 'package:CookingApp/widgets/appbar_widget.dart';
import 'package:CookingApp/widgets/loading.dart';
import 'package:CookingApp/widgets/raised_button_widget.dart';
import 'package:CookingApp/widgets/text.dart';
import 'package:CookingApp/widgets/textfield_blocbuilder.dart';
import 'package:flutter/material.dart';
import 'package:flutter_form_bloc/flutter_form_bloc.dart';

class LogInPage extends StatefulWidget {
  @override
  _LogInPageState createState() => _LogInPageState();
}

class _LogInPageState extends State<LogInPage> {
  LogInFormBloc _logInFormBloc;

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      create: (context) {
        _logInFormBloc = LogInFormBloc();
        return _logInFormBloc;
      },
      child: FormBlocListener<LogInFormBloc, String, String>(
        onSuccess: (context, state) {
          print(state.successResponse);
          LoadingDialog.hide(context);
          Navigator.pushNamed(context, Routing.HomePageRoute);
        },
        onFailure: (context, state) {
          print(state.failureResponse);
          LoadingDialog.hide(context);
        },
        onSubmitting: (context, state) {
          LoadingDialog.show(context);
        },
        child: SafeArea(
          child: Scaffold(
            appBar: AppBarWidget(),
            body: SingleChildScrollView(
              child: Center(
                child: Column(
                  children: <Widget>[
                    TextFieldBlocBuilderWidget(
                        _logInFormBloc.userNameField,
                        "Username",
                        true,
                        TextInputType.text,
                        false,
                        Icons.person),
                    TextFieldBlocBuilderWidget(_logInFormBloc.passwordField,
                        "Password", true, TextInputType.text, true, Icons.lock),
                    Container(
                      width: 200,
                      height: 50,
                      child: RaisedButtonWidget(
                          TextWidget("LOG IN"), _logInFormBloc.submit),
                    )
                  ],
                ),
              ),
            ),
          ),
        ),
      ),
    );
  }

  @override
  void dispose() { 
    _logInFormBloc.close();
    super.dispose();
  }
}

@felangel
Copy link
Owner

You should be accessing properties on the bloc state rather than the bloc itself. I would highly recommend reading through the documentation and in this case use BlocBuilder to access the usernameField.

@jaydangar
Copy link
Author

jaydangar commented Jun 19, 2020

@felangel Actually I am using Form_Bloc library, which requires to provide bloc as a parameter. See the following code:

import 'package:flutter/material.dart';
import 'package:flutter_form_bloc/flutter_form_bloc.dart';

class TextFieldBlocBuilderWidget extends StatefulWidget {

  final TextFieldBloc _bloc;
  final String _hint;
  final bool _autocorrect;
  final bool _obscureText;
  final TextInputType _inputType;
  final IconData _prefixIcon;
  
  TextFieldBlocBuilderWidget(this._bloc,this._hint,this._autocorrect,this._inputType,this._obscureText,this._prefixIcon);

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

class _TextFieldBlocBuilderWidgetState extends State<TextFieldBlocBuilderWidget> {

  InputDecoration _inputDecoration;

  @override
  void initState() {
    _inputDecoration = InputDecoration(labelText: widget._hint,border: OutlineInputBorder(),prefixIcon: Icon(widget._prefixIcon));
    super.initState();
  }

  @override
  Widget build(BuildContext context) {

    var _obscureText;

    if(widget._obscureText){
      _obscureText = SuffixButton.obscureText;
    }

    return TextFieldBlocBuilder(
      textFieldBloc: widget._bloc,
      decoration: _inputDecoration,
      autocorrect: widget._autocorrect,
      keyboardType: widget._inputType,
      obscureText: widget._obscureText,
      suffixButton: _obscureText,
    );
  }
}

Here as you can see TextFieldBlocBuilder is a builder method, by which the widget gets updated. So, I have a different requirnment. Can you kindly let me know if there's any effective solution for this?

I am adding a link to TextFieldBlocBuilder class Documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants