-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[google_maps_flutter] Raise MapUsedAfterWidgetDisposedError when map controller used after map disposed
#9242
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
[google_maps_flutter] Raise MapUsedAfterWidgetDisposedError when map controller used after map disposed
#9242
Conversation
|
I was just made aware of #9227, which seems to fix the other aspect of flutter/flutter#43785. Happy to wait with this one until after that one lands. |
|
From triage: Still waiting on resolution of #9227 first. |
|
Skipping stale PR nudge per blocked PR. :) |
|
Should be unblocked very shortly. |
|
This is unblocked now. FYI, the new file will need a fresh copy of the license header when you merge in |
3b53dc2 to
c3eac1e
Compare
|
I've rebased and updated the code. This is now ready for review. |
9f01e7a to
90ceed1
Compare
| /// 1. Set the map controller field to `null` in your widget state's | ||
| /// `dispose()` method, or | ||
| /// 2. Check the [State.mounted] state before each use of the controller. | ||
| class MapUsedAfterWidgetDisposedError extends Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a compelling reason to create a new Error type rather than use StateError with the same string used below as its message? A private helper method that returns that state error would still avoid code duplication, but without an extra class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it nicer to have a separate class at the time, but I'm happy to just use StateError. I'll make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
| @@ -1,3 +1,8 @@ | |||
| ## 2.13.2 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New API is a minor version bump, per semver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
Sorry for the delay. The custom error class is gone and I bumped to 2.14.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
From triage: @filiph is this waiting on anything before it lands? |
Not at all! It can land immediately. |
* 'main' of https://github.com/CaoGiaHieu-dev/packages: Update packages/go_router/CHANGELOG.md [tool] Remove use of FETCH_HEAD (flutter#10357) Roll Flutter from 027f2e410241 to e5d5c01850f2 (73 revisions) (flutter#10362) [camera_platform_interface] Adds support for video stabilization to camera_platform_interface (flutter#10337) [google_maps_flutter] Raise `MapUsedAfterWidgetDisposedError` when map controller used after map disposed (flutter#9242) [pigeon] Replace containsKey with contains in Kotlin generator (flutter#10274) [video_player] Remove `package` in example `AndroidManifest.xml` file (flutter#10245) [flutter_svg] Fixes typo of `allowDrawingOutsideViewBox` in doc comments. (flutter#10256) [in_app_purchase] Remove use of Pigeon's Dart test generator (flutter#10328) [dependabot]: Bump com.squareup.okhttp3:okhttp from 5.1.0 to 5.3.0 in /packages/espresso/android (flutter#10348) Roll Flutter from 6f8abdd77820 to 027f2e410241 (26 revisions) (flutter#10335) [google_sign_in] Remove use of OCMock (flutter#10290) [interactive_media_ads] Pin iOS dependency maximum (flutter#10349)
…r` when map controller used after map disposed (flutter/packages#9242)
…r` when map controller used after map disposed (flutter/packages#9242)
…r` when map controller used after map disposed (flutter/packages#9242)
This change introduces a new
MapUsedAfterWidgetDisposedErrorthat facilitates debugging when the user calls a method onGoogleMapsControllerafter the associated map has been disposed. This replaces the previous behavior, which would sometimes throw a Platform-side error (MissingPluginException,Unable to establish connection on channel, etc.).Although technically non-breaking (we're replacing one error with another), this could be disruptive because the new error is raised eagerly and synchronously, as soon as a call is made. Before, the call could have potentially succeeded (?), and the error was asynchronous (and so, potentially, unawaited and ignored?).
Fixes flutter/flutter#43785.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3