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

Incomplete or missing didUpdateWidget method #58333

Open
jacob314 opened this issue Feb 28, 2021 · 2 comments
Open

Incomplete or missing didUpdateWidget method #58333

jacob314 opened this issue Feb 28, 2021 · 2 comments
Labels
area-devexp Developer Experience related issues (DevTools, IDEs, Analysis Server, completions, refactoring, etc) customer-flutter devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@jacob314
Copy link
Member

Flutter developers are unable to hot reload as many changes to the parameters of StatefulWidgets as they should because didUpdateWidget is often not implement or not implement fully. Apps can also suddenly start failing in confusing ways due to changes at a distance that cuase Flutter to reuse State objects where it didn’t before exposing these bugs.

Describe the rule you'd like to see implemented
The lint should trigger if a Flutter State object's initState method stores values from fields of a widget but didUpdateWidget does not at minimum have a code path that reads those same fields. This will require flow analysis within the State class as it is common to have helper methods called from both initState and didUpdateWidget that update the State object to match the widget. We will need to carefully track how many false positives this lint triggers. If the flow analysis is robust there should be few very few positives but there may be some false negatives where the lint thinks didUpdateWidget methods are fine even though they have bugs as we cannot ensure the logic in didUpdateWidget is correct only that it is not neglecting members from the widget. If it doesn't introduce significant false positives, we could warn about absolutely any member use in initState that is not also matched by member use in didUpdateWidget.

Examples

All of these examples are members of a State object

 @override
 void initState() {
   super.initState();
   _controller = AnimationController(vsync: tickerProvider);
   _updateAnimationController(); 
 }
 

 
 @override
 void didUpdateWidget(SingleTimerWidget oldWidget) {
   super.didUpdateWidget(oldWidget);
  // Lint if this block is missing as _timer is referenced in updateAnimationController that is called from initState
   if (widget._timer != oldWidget._timer) { // P2 probably false positive prone lint would be to warn if there aren't checks for if the oldWidget and newWidget have an equal field.
   _updateAnimationController()
   }
  }
 
  void _updateAnimationController() {
    final remainingProgress = _toProgress(widget._timer); 
  }
 }

No lint error in this case. There is an initState but no didUpdateWidget method but that does not indicate a bug as initState does not read any data off the widget.

 @override
 void initState() {
   super.initState();
   _controller = AnimationController(vsync: tickerProvider);
 }

Ideally, provide motivating examples w/ rationale.

Additional context
We find in practice that users very frequently neglect to implement didUpdateWidget as issues tend to only show up on a hot reload or rare cases where the same Widget object

@jacob314 jacob314 added type-enhancement A request for a change that isn't a bug linter-lint-request labels Feb 28, 2021
@jacob314
Copy link
Member Author

Fyi @pq filed an issue for this case we have been discussing. I expect this lint should find a very large number of real issues.

@jacob314 jacob314 changed the title incomplete or missing didUpdateWidget method Incomplete or missing didUpdateWidget method Feb 28, 2021
@stan-at-work
Copy link

stan-at-work commented Nov 16, 2024

They could be realy great

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
@bwilkerson bwilkerson added area-devexp Developer Experience related issues (DevTools, IDEs, Analysis Server, completions, refactoring, etc) and removed area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp Developer Experience related issues (DevTools, IDEs, Analysis Server, completions, refactoring, etc) customer-flutter devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants