Skip to content

Conversation

@goderbauer
Copy link
Member

@goderbauer goderbauer commented Sep 14, 2022

This makes it a little easier to use BuildContext error-free across asynchronous gaps in StatelessWidgets and other instances where you can't check State.mounted.

Fixes #111488

DO NOT MERGE until use_build_context_synchronously lint has been updated to recognize BuildContext.mounted: dart-lang/sdk#58872

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Sep 14, 2022
/// an asynchronous operation), consider checking [mounted] to determine whether
/// the context is still valid before interacting with it:
///
/// ```dart
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost a functional example app. Would it make sense to make it into a full sample?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it doesn't demonstrate anything visually though. In this case I think a full sample would actually distract from the point this is trying to make...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, makes sense. I mean it could show a dialog going away after a second, but I agree it's not all that visual.

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

Copy link
Contributor

@fzyzcjy fzyzcjy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an expert here, but I see State.mounted is sometimes not enough for a State.context to be used.

What I have seen: error, Looking up a deactivated widget's ancestor is unsafe., when using SomeInheritedWidget.of(context) - you know, the very common practice. The error is like

Looking up a deactivated widget's ancestor is unsafe.
At this point the state of the widget's element tree is no longer stable.
To safely refer to a widget's ancestor in its dispose() method, save a reference to the ancestor by calling dependOnInheritedWidgetOfExactType() in the widget's didChangeDependencies() method.

When the exception was thrown, this was the stack: 
#0      Element._debugCheckStateIsActiveForAncestorLookup.<anonymous closure> (package:flutter/src/widgets/framework.dart:3990:9)
#1      Element._debugCheckStateIsActiveForAncestorLookup (package:flutter/src/widgets/framework.dart:4004:6)
#2      Element.dependOnInheritedWidgetOfExactType (package:flutter/src/widgets/framework.dart:4019:12)
#3      SomeOfMyInheritedWidget.of (...)

That seems to come from when the widget is deactivated and not yet (re) activated.

My personal workaround is that, has an extra variable tracking activate/deactivate, and never call them when it is deactivated and not yet activated. But that does not sound elegant.

Notice it uses _debugCheckStateIsActiveForAncestorLookup, and your example code, Navigator.of(context), calls findRootAncestorStateOfType which also has the _debugCheckStateIsActiveForAncestorLookup assertion.
Therefore, I suspect this PR will suffer from the same error.

/// onPressed: () async {
/// await Future<void>.delayed(const Duration(seconds: 1));
/// if (context.mounted) {
/// Navigator.of(context).pop();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(here)

@goderbauer
Copy link
Member Author

@fzyzcjy Can you file an issue for that? Ideally with some code that reproduces the error you're seeing? I am curious to see how you can do a dependency lookup while a global key move or reparenting is in progress.

@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 15, 2022
@auto-submit auto-submit bot merged commit b8df71f into flutter:master Sep 15, 2022
@fzyzcjy
Copy link
Contributor

fzyzcjy commented Sep 15, 2022

@goderbauer Sure I will do that when see the bug again

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2022
@rhydiant
Copy link

rhydiant commented Oct 26, 2022

@goderbauer - is it possible to see which Flutter release this will end up in? I'm getting use_build_context_synchronously lint warnings with Flutter 3.3.5 (channel stable) and the docs refer to BuildContext.mounted but it doesn't appear to be supported yet

@ashishbeck
Copy link
Contributor

I can see this is currently available in the beta branch but not stable as of 3.3.6 Do we have a timeline for this in stable?

@ChangJoo-Park
Copy link

BuildContext.mounted does not appear in 3.3.5

@ValentinVignal
Copy link
Contributor

BuildContext.mounted does not appear in 3.3.5

@ChangJoo-Park I believe we will need to wait for a least the next minor version (>= 3.4.0)

@Troyx21
Copy link

Troyx21 commented Nov 17, 2022

It's been TWO MONTHS after this was merged and still linter warnings are present, linking to the invalid docs page. Why should this wait for the next minor release, shouldn't this have been the immediate patch (bugfix)? This should be considered bugfix, as linter page is a bug (it provides solution that doesn't build).

@gspencergoog
Copy link
Contributor

@Troyx21 This feature is available in the beta and master channels. Please take a look at the description of our release channels to decide if you want to switch to one of those channels, but we typically only release to the stable channel once a quarter or so (every three months), so it is expected for a change to take about that long to get to the stable channel from the time it is committed to master. If you absolutely need the feature sooner than that, you are welcome to switch to a less stable channel in the meantime.

@Troyx21
Copy link

Troyx21 commented Nov 17, 2022

@gspencergoog I do understand that perfectly well. What I tried to communicate, is that this particular issue, because of the broken linter page and annoing linter warning that affects probably thouthands of codebases, could have been identified as a bug, therefore - this PR could have been labeled as a bugfix - and hence merged into stable as soon as the next bugfix (patch) version was issued (3.3.X).

@gspencergoog
Copy link
Contributor

I'm sorry that it's annoying to you, I truly am. However, we are very unlikely to elevate any annoyance issue to the level of a cherrypick/hotfix into the stable channel because the risk to the stability of the stable channel is too high. We set the bar very high for those hotfixes because we want you to have confidence in the stability of the stable channel, as most developers would rather have annoying linter messages than angry customers. As this page mentions, you are also welcome to patch your local version of Flutter if you must be on the stable channel but need the fix sooner.

@guplem
Copy link

guplem commented Dec 17, 2022

Is context.mounted already available in version 3.3.10? I am getting the error The getter 'mounted' isn't defined for the type 'BuildContext'.

I believe we will need to wait for a least the next minor version (>= 3.4.0)

@ValentinVignal Is this confirmed anywhere? I am searching for information about the expected release dates & changes of the versions, but I can't find it… 3 months after the merge "seem enough" to release it into the stable version…

This is my flutter doctor output:
[√] Flutter (Channel stable, 3.3.10, on Microsoft Windows [Version 10.0.22621.819], locale en-150)
[√] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
[√] Chrome - develop for the web
[√] Visual Studio - develop for Windows (Visual Studio Build Tools 2019 16.11.19)
[√] Android Studio (version 2021.3)
[√] VS Code (version 1.74.1)
[√] Connected device (3 available)
[√] HTTP Host Availability
• No issues found!

This is my code:

loadConfigurationFromLocalStorage(BuildContext context) async {
  final localStorage = await SharedPreferences.getInstance();

  if (!context.mounted) {  // <-- Here I get the error 
    print("Context not mounted while trying to load configuration from local storage");
    return;
  }

  final String? storedLanguage = localStorage.getString("lang");
  if (storedLanguage != null) {
    Provider.of<AppProvider>(context, listen: false).setLanguage(storedLanguage);
  }

}

It might be the case that I am using it wrong, but if this is not a proper way of loading data from the local storage and saving it to a provider, how should I do it?

@ValentinVignal
Copy link
Contributor

@guplem I don't belong to the flutter team so everything I'm saying is not confirmed. It's only suppositions.

Is context.mounted already available in version 3.3.10? I am getting the error The getter 'mounted' isn't defined for the type 'BuildContext'.

Since context.mounted is a new feature, I doubt they will release it in a patch version (3.3.x), those are reserved for hotfixes. It will be at least a new minor version (>= 3.4.0) (as I said in my comment #111619 (comment)) You can read more about semantic versioning.

3 months after the merge "seem enough" to release it into the stable version…

Flutter releases a new stable version "roughly once a quarter" , so a new stable version should be released soon (but I don't know when). You can read more about the Flutter build release channels.
But there is also no guarantee context.mounted will be included in the next stable release (ex: 3.7.0).

@rupinderjeet
Copy link

Until then, can something like this work?

extension BuildContextMountedExt on BuildContext {
  bool get mounted {
    try {
      widget;
      return true;
    } catch (e) {
      return false;
    }
  }
}

@ValentinVignal
Copy link
Contributor

It looks like it has been released with Flutter 3.7.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose a mounted-like flag on BuildContext