dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.3k stars 1.59k forks source link

proposal: `use_set_state_synchronously` #59410

Open navaronbracke opened 9 months ago

navaronbracke commented 9 months ago

use_set_state_synchronously

I propose a new lint, akin to its counterpart, use_build_context_synchronously, but for setState() invocations.

Description

Use setState() synchronously and guard it with a mounted check if needed.

Details

Using setState() after an async gap is potentially unsafe. Consider guarding it with a mounted check.

We should consider both explicit awaits and sync/async versions of the then/catchError/whenComplete callbacks.

Kind

It guards against errors.

Bad Examples

await Future.value(42);

setState(() {});
await Future.value(42).then((int value) {
  setState(() {});
});
await Future.value(42).then((int value) async {
  await Future<void>.delayed(const Duration(seconds: 1));

  setState(() {});
});
await Future.value(42).catchError((Object error, StackTrace stackTrace) {
  setState(() {});
});
await Future.value(42).whenComplete(() {
  setState(() {});
});

Good Examples

await Future.value(42);

if (mounted) {
  setState(() {});
}
await Future.value(42);

if (!mounted) {
  return;
}

setState(() {});
await Future.value(42);

if (!context.mounted) {
  return;
}

setState(() {});
await Future.value(42).then((int value) {
  if (!mounted) {
    return;
  }

  setState(() {});
});
await Future.value(42).then((int value) async {
  await Future<void>.delayed(const Duration(seconds: 1));

  if (!mounted) return;

  setState(() {});
});
await Future.value(42).catchError((Object error, StackTrace stackTrace) {
  if (!mounted) {
    return;
  }

  setState(() {});
});
await Future.value(42).whenComplete(() {
  if (!mounted) {
    return;
  }

  setState(() {});
});

Discussion

I have seen bugs (both in the framework and in user code) due to bad usages of setState() after an async gap.

I.e. https://github.com/flutter/flutter/issues/124597

Discussion checklist

srawlins commented 9 months ago

CC @goderbauer

goderbauer commented 9 months ago

Nice suggestion. I think this would be a good one to have. setState and using context after an async gap should follow the same rules. I wonder how many violations of this we have in the framework...

pq commented 9 months ago

@stereotype441 (re: flow analysis)