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

BuildContext across async gaps with mounted check false positive #5069

Closed
vendelin8 opened this issue Aug 20, 2024 · 8 comments
Closed

BuildContext across async gaps with mounted check false positive #5069

vendelin8 opened this issue Aug 20, 2024 · 8 comments
Labels
set-flutter Affects a rule in the recommended Flutter rule set

Comments

@vendelin8
Copy link

Describe the issue
Lint error use_build_context_synchronously when it was already checked

To Reproduce

    TextButton(
      child: Text('Choose a file...'),
      onPressed: () async {
        FilePickerResult? result = await FilePicker.platform.pickFiles(
          dialogTitle: 'Choose a file',
          allowCompression: true,
          type: FileType.image,
        );
        if (!mounted) return;

        if (result == null) {
          Navigator.of(context).pop(); // LINT INFO HERE
          return;
        }
        Navigator.of(context).pop(await cp.getImage(result.files.single)); // LINT INFO HERE
      },
    ),

Expected behavior
No lint infos, because mounted was already checked previously.

Additional context

dart --version
Dart SDK version: 3.5.0 (stable) (Tue Jul 30 02:17:59 2024 -0700) on "linux_x64"
@github-actions github-actions bot added the set-flutter Affects a rule in the recommended Flutter rule set label Aug 20, 2024
@srawlins
Copy link
Member

srawlins commented Aug 20, 2024

Is TextButton the containing class a State subclass? If not, you need to do a mounted check with context.mounted, not mounted.

@vendelin8
Copy link
Author

This is a function Future<void> newPhoto() async in class _ProfileImageState extends State<_ProfileImage> which is the state of class _ProfileImage extends StatefulWidget

@srawlins
Copy link
Member

I see. Can you provide a full reproduction? I cannot reproduce this issue in unit tests.

@vendelin8
Copy link
Author

@srawlins
Copy link
Member

@goderbauer could you clarify for me, in this code:

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) => const Scaffold(
        body: Text(
          'You have pushed the button this many times:',
        ),
      );

  Future<int?> test() async {
    await Future.delayed(const Duration(seconds: 1));
    return 1;
  }

  Future<void> newPhoto() async {
    await showDialog(
        context: context,
        builder: (BuildContext context) => AlertDialog(
              actions: [
                TextButton(
                  onPressed: () async {
                    int? result = await test();
                    if (!mounted) return;
                    Navigator.of(context).pop(result);
                  },
                ),
              ],
            ));
  }
}

(simplified from https://github.com/vendelin8/dart-lintbug/blob/main/lib/main.dart)

Key points:

  • We are in a subclass of State
  • We are in a builder function that has a local context variable
  • We want to call await foo, then call Navigator.of(context)

Do we need to guard that use of the local context variable via the surrounding State.mounted (this.mounted) or with context.mounted?

@goderbauer
Copy link
Contributor

Do we need to guard that use of the local context variable via the surrounding State.mounted (this.mounted) or with context.mounted?

You are using the local context in Navigator.of(context) so you want to guard on that since that local context and the surrounding State can have different life cycles.

@srawlins
Copy link
Member

OK good, the docs are accurate on this question. Thanks!

  • When using a State's context property, the State's mounted property must be checked.
  • For other BuildContext instances (like a local variable or function argument), the BuildContext's mounted property must be checked.

@vendelin8
Copy link
Author

Ok, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
set-flutter Affects a rule in the recommended Flutter rule set
Projects
None yet
Development

No branches or pull requests

3 participants