Skip to content

use_build_context_synchronously is triggered when using context.mounted #59009

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

Closed
luccasclezar opened this issue Jan 25, 2023 · 15 comments · Fixed by dart-archive/linter#4134 or dart-archive/linter#4312
Assignees
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-false-positive linter-set-flutter P2 A bug or feature request we're likely to work on

Comments

@luccasclezar
Copy link

After upgrading to Flutter 3.7, using BuildContext.mounted in any way other than if (context.mounted) {} makes the linter complain about using context across async gaps.

@goderbauer told me to mention him.

Reproduction steps

  1. Create a new project with flutter create
  2. Add this Widget to main.dart
class AsyncContextTestWidget extends StatelessWidget {
  const AsyncContextTestWidget({super.key});

  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
      onPressed: () async {
        await Future<void>.delayed(Duration.zero);

        // This line is marked as using context across async gaps
        if (!context.mounted) {
          return;
        }
      },
      child: const Text('Test'),
    );
  }
}

!context.mounted is just one example of this problem. This is reproducible when using ternary operators or initializing a variable to context.mounted too.

// This line triggers a warning too
final isMounted = context.mounted;

if (!isMounted) {
  return;
}
@andrewescutia
Copy link

If it helps here is an example failing test:

void methodWithBuildContextParameter4(BuildContext context) async { 
  await Future<void>.delayed(Duration());
  if (!context.mounted) {
    return;
  }
  await Navigator.of(context).pushNamed('routeName'); // OK
}

The added second if check below within lib/src/rules/use_build_context_synchronously.dart appears to correct it but I do not have an intimate knowledge of this repo.

        // if (mounted) { ... do ... }
        if (isMountedCheck(parent, positiveCheck: true)) {
          return;
        }

        // if (!context.mounted) { return; }
        if (isMountedCheck(parent)) {
          return;
        }

@LostInDarkMath
Copy link

This bug is pretty anoying. Hope you can fix this soon :)

@asashour
Copy link
Contributor

asashour commented Feb 2, 2023

For fixing this, should we search for specific statements, e.g. if (!context.mounted) return; or we should use FlowAnalysis, to check that context is not used before .mount, in the same function/method?

@pq
Copy link
Member

pq commented Feb 2, 2023

It's unfortunate that we're approximating what flow analysis does more completely but I'm not aware of any way to leverage it in a lint implementation (though it has been discussed).

/fyi @stereotype441

@Levi-Lesches
Copy link

Just ran into this issue today, pretty unintuitive why

if (!context.mounted) return;

was failing but

if (context.mounted) doSomething(context)

wasn't

@luccasclezar
Copy link
Author

@Levi-Lesches While this is not fixed, if you really want to return instead of placing more code inside an if, you can use:

if (context.mounted) {
} else {
  return;
}

It's not great, but sometimes it can be better than nesting ifs, plus when this is fixed the refactoring will be pretty quick.

@haslinger
Copy link

Cool tip, @luccasclezar! And to be able fo find these instances later, I modified it to

  if (context.mounted) {
    // FIXME
  } else {
    return;
  }

@JosephNK
Copy link

JosephNK commented Apr 8, 2023

I hope it will be corrected. :)

@gnprice
Copy link
Contributor

gnprice commented May 4, 2023

I'm continuing to see this issue as of a Dart SDK version from today (from the Flutter main/master channel):

$ dart --version
Dart SDK version: 3.1.0-75.0.dev (dev) (Thu May 4 03:59:43 2023 -0700) on "linux_x64"

In particular, if I take a test case from the PR dart-archive/linter#4312:

import 'package:flutter/widgets.dart';

void foo(BuildContext context) async {
  await f();
  if (!context.mounted) {
    return;
  }
  Navigator.of(context);
}

Future<void> f() async {}

and run dart analyze, I get the lint message:

$ dart analyze lib/tmp.dart
Analyzing tmp.dart...                  0.5s

   info • tmp.dart:5:8 • Don't use 'BuildContext's across async gaps. Try
          rewriting the code to not reference the 'BuildContext'. •
          use_build_context_synchronously

1 issue found.

Is that unexpected, or is there something I'm missing?

@pq
Copy link
Member

pq commented May 4, 2023

@srawlins

@pq pq reopened this May 4, 2023
@srawlins
Copy link
Member

srawlins commented May 4, 2023

Dart SDK at head is at linter e36813b which does not include this fix.

@srawlins srawlins closed this as completed May 4, 2023
@gnprice
Copy link
Contributor

gnprice commented May 4, 2023

Good to know, thanks.

What's the typical cadence of when new linter versions enter the Dart SDK? The readme for this repo just says:

The linter is bundled with the Dart SDK; if you have an updated Dart SDK already, you're done!

It's perfectly reasonable that it doesn't propagate instantly; I'm just curious what to expect.

@gnprice
Copy link
Contributor

gnprice commented May 4, 2023

Relatedly: is there a straightforward way to look up what linter version is in the SDK? It'd be great to document that in the readme file too, if possible.

@srawlins
Copy link
Member

srawlins commented May 4, 2023

All good questions!

I'd say we're aspiring for continuous deployment; each commit here should land ASAP in the Dart SDK. But we're definitely not there yet, commits here typically make it over to the Dart SDK in under a month.

Relatedly: is there a straightforward way to look up what linter version is in the SDK? It'd be great to document that in the readme file too, if possible.

We use a homegrown vendoring system, so you'll see the commit SHA in this file: https://github.com/dart-lang/sdk/blob/main/DEPS#L147

gnprice referenced this issue in zulip/zulip-flutter May 10, 2023
This workaround recently stopped working: this inverted form of
early return no longer persuades the lint rule not to complain
when we go on to use `context`.

Not sure exactly what change caused that; it happened with a new
Flutter version from master/main in recent weeks, presumably due
to a new Dart SDK version rolling through there.

Since the workaround is no longer effective, just switch to the
straightforward early return, and use a lint-ignore comment
pointing at the underlying issue that causes the lint rule to
complain there:
  https://github.com/dart-lang/linter/issues/4007

That issue was recently fixed in the linter repo's main branch.
The fix rolled yesterday into the Dart SDK:
  dart-lang/sdk@f09ce66

but that hasn't quite yet rolled through to Flutter.
Once it does, we'll be able to cut these.
pattersonjack referenced this issue in pattersonjack/liftoff Jul 16, 2023
This includes a workaround for mobx stores:
mobxjs/mobx.dart#809

Also, while uuid was already imported in fullscreenable_image.dart, it was not in pubspec.yaml.

Lastly, there are some linting issues with use_build_context_synchronously. For the files most heavily impacted, I have added a temporary ignore rule as the linting fixes are not currently in the Flutter SDK. See: https://github.com/dart-lang/linter/issues/4007
pattersonjack referenced this issue in pattersonjack/liftoff Jul 17, 2023
This includes a workaround for mobx stores:
mobxjs/mobx.dart#809

Also, while uuid was already imported in fullscreenable_image.dart, it was not in pubspec.yaml.

Lastly, there are some linting issues with use_build_context_synchronously. For the files most heavily impacted, I have added a temporary ignore rule as the linting fixes are not currently in the Flutter SDK. See: https://github.com/dart-lang/linter/issues/4007
shocklateboy92 referenced this issue in liftoff-app/liftoff Jul 17, 2023
Adds flutter_lints, removes duplicate rules, and addresses linting
issues. See #232 for motivation.

This includes a
[workaround](mobxjs/mobx.dart#809) for mobx
stores.

Also, while the uuid package was already imported in
fullscreenable_image.dart, it was not in pubspec.yaml.

Lastly, there are some issues with the use_build_context_synchronously
rule. For the files most heavily impacted, I have added a temporary
ignore rule as the linting fixes are not currently in the Flutter SDK.
See: https://github.com/dart-lang/linter/issues/4007
pattersonjack referenced this issue in pattersonjack/liftoff Jul 18, 2023
This includes a workaround for mobx stores:
mobxjs/mobx.dart#809

Also, while uuid was already imported in fullscreenable_image.dart, it was not in pubspec.yaml.

Lastly, there are some linting issues with use_build_context_synchronously. For the files most heavily impacted, I have added a temporary ignore rule as the linting fixes are not currently in the Flutter SDK. See: https://github.com/dart-lang/linter/issues/4007
pattersonjack referenced this issue in pattersonjack/liftoff Jul 18, 2023
This includes a workaround for mobx stores:
mobxjs/mobx.dart#809

Also, while uuid was already imported in fullscreenable_image.dart, it was not in pubspec.yaml.

Lastly, there are some linting issues with use_build_context_synchronously. For the files most heavily impacted, I have added a temporary ignore rule as the linting fixes are not currently in the Flutter SDK. See: https://github.com/dart-lang/linter/issues/4007
@Wellcomezhangheng
Copy link

image

Take away without thanking

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-false-positive linter-set-flutter P2 A bug or feature request we're likely to work on
Projects
None yet