Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

[New rule] avoid-unnecessary-setstate #332

Closed
incendial opened this issue May 13, 2021 · 3 comments · Fixed by #346
Closed

[New rule] avoid-unnecessary-setstate #332

incendial opened this issue May 13, 2021 · 3 comments · Fixed by #346
Assignees
Labels
type: enhancement New feature or request
Milestone

Comments

@incendial
Copy link
Member

incendial commented May 13, 2021

Please describe what the rule should do:

As @GroovinChip pointed out in one of a twitter thread, calling setState from a widget initState or build methods will lead to unnecessary rerender of the widget. It looks like a potential problem especially for new developers, so the idea of the rule is to warn about setState invocations in initState or build.

We have not quite come to a conclusion, should we also consider calling sync methods from a widget initState with setState call in it bad or acceptable.

@GroovinChip thanks again for providing examples and sharing the idea.

Additional resources:

What category of rule is this? (place an "X" next to just one item)

[X] Warns about a potential error (problem)
[ ] Suggests an alternate way of doing something (suggestion)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about (it will be better if you can provide both good and bad examples):
Bad:

class MyWidget extends StatefulWidget {
  @override
  _MyWidgetState createState() => _MyWidgetState();
}

class _MyWidgetState extends State<MyWidget> {
  String myString = '';
  
  @override
  void initState() {
    super.initState();
   
    setState(() {
      myString = "Hello";
    }); // Bad
    
    if (condition) {
      setState(() {}); // Bad
    }
  }
  
  @override
  Widget build(BuildContext context) {
    setState(() {
      myString = "Hello";
    }); // Bad
    
    if (condition) {
      setState(() {}); // Bad
    }

    return ElevatedButton(
      child: Text('PRESS'),
    );
  }
}
class MyWidget extends StatefulWidget {
 @override
  _MyWidgetState createState() => _MyWidgetState();
}

class _MyWidgetState extends State<MyWidget> {
  String myString = '';
  
  @override
  void initState() {
    super.initState();
    myMethod(); // Good or Bad ???
    myAsyncMethod(); // Good
  }
  
  void myMethod() {
    setState((){
      myString = "Hello";
    }); // Good
  }
 
  Future<void> myAsyncMethod() async {
    final data = await service.fetchData();
      
    setState(() {
      myString = data;
    });
  }
  
  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
      onPressed: () => myMethod(), // Good
      child: Text('PRESS'),
    );
  }
}
@incendial incendial added the type: enhancement New feature or request label May 13, 2021
@incendial
Copy link
Member Author

incendial commented May 13, 2021

@GroovinChip if you have some time to review the proposal, I'd be very grateful.

@dkrutskikh @roman-petrov I'd really happy to hear your thoughts about this rule and maybe you know some other places where calling setState should be considered unnecessary.

I'm not quite sure how this rule should be called, right now we have 3 options:

  • avoid-unnecessary-setstate
  • avoid-additional-render
  • avoid-rerender

If you have any opinion on that, I'd like to hear it as well.

Also, if you know anyone, who can add some thoughts on this rule, please invite them to the discussion.

@incendial
Copy link
Member Author

Hm, that looks like the same problem flutter/devtools#1336 (comment)

@GroovinChip
Copy link

I'll try to review tonight or tomorrow, thanks @incendial

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants