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

[BUG] External MapController lifecycle not synced with widget when destroyed and rebuilt #1892

Closed
Zverik opened this issue May 27, 2024 · 30 comments · Fixed by #1915
Closed
Labels
bug This issue reports broken functionality or another error P: 2 (soon™?) S: core Scoped to the core flutter_map functionality

Comments

@Zverik
Copy link
Contributor

Zverik commented May 27, 2024

What is the bug?

I have a FlutterMap in a ListView. It displays fine. When I scroll down so the map is not visible (and the state is lost), and then scroll back up, the map is broken (at least in the debug build), and there is an assertion in the console: "Should not update options unless they change", originating from map_controller_impl.dart:342.

I guess the widget gets deleted when not in view, then built again, and the same mapController gets reattached. Is that correct?

How can we reproduce it?

Sorry I don't have a short code at hand, my laptop acts up. I encountered this when running Every Door: find a POI and tap on it, and then scroll down, opening "more fields", to the bottom. And then up. Here is how the object created.

Do you have a potential solution?

Remove the assertion?..

Platforms

Android 14 (Sony Xperia 10 IV).

Severity

Erroneous: Prevents normal functioning and causes errors in the console

@Zverik Zverik added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels May 27, 2024
@JaffaKetchup

This comment was marked as off-topic.

@Zverik
Copy link
Contributor Author

Zverik commented May 28, 2024

Well it worked with version 6.1.0, but broke with 7.0.0.

Tested with a production build: the map disappears, and there is an exception in the log:

[07:35:32] Flutter: LateInitializationError: Field '_interactiveViewerState@832162146' has already been initialized.
[07:35:32] #0      LateError._throwFieldAlreadyInitialized (dart:_internal-patch/internal_patch.dart:185)
#1      MapControllerImpl._interactiveViewerState= (package:flutter_map/src/map/controller/map_controller_impl.dart)
#2      MapControllerImpl.interactiveViewerState= (package:flutter_map/src/map/controller/map_controller_impl.dart:47)
#3      MapInteractiveViewerState.initState (package:flutter_map/src/gestures/map_interactive_viewer.dart:103)
#4      StatefulElement._firstBuild (package:flutter/src/widgets/framework.dart:5618)
#5      ComponentElement.mount (package:flutter/src/widgets/framework.dart:5463)
...

@JaffaKetchup
Copy link
Member

That is something we weren't expecting to see. I wasn't aware that anything had changed in that area of the code.

@JaffaKetchup
Copy link
Member

@Zverik I noticed you're initialising your map controller within the build method: https://github.com/Zverik/every_door/blob/6026f164baf2b66c067898be24f7205a1d05fd74/lib/screens/editor.dart#L547. This could be causing issues. Can you try moving it out? (https://docs.fleaflet.dev/usage/programmatic-interaction/external-custom-controllers)

@Zverik
Copy link
Contributor Author

Zverik commented May 28, 2024

Alas no, it's declared outside any of methods :( Would be an easy fix!

@JaffaKetchup
Copy link
Member

Ah, misread the indentation. The only way this can occur is when the initState method is run on the MapInteractiveViewer multiple times, and the map controller remains the same.

@JaffaKetchup JaffaKetchup added P: 1 (important) S: core Scoped to the core flutter_map functionality and removed needs triage This new bug report needs reproducing and prioritizing labels May 28, 2024
@JaffaKetchup
Copy link
Member

I'm thinking this was introduced in #1738. It made the most changes to this area between v6 and v7.

@Zverik
Copy link
Contributor Author

Zverik commented May 28, 2024

So I took the editor.dart and shortened it piece by piece until I got a minimal example for the bug: https://gist.github.com/Zverik/d056509e22eeecffdefb3a42a587325f

When I comment out the mapController assignment, it works.

@Zverik
Copy link
Contributor Author

Zverik commented May 28, 2024

I frankly do not understand the nature of changes in lib/src/map/widget.dart, but I feel like options should be checked for change before reassignment. The widget was disposed, then re-created, but the options (and the map controller) are the same, since the state preserved them.

@JaffaKetchup
Copy link
Member

@Zverik As a short term workaround, try setting keepAlive to true in MapOptions. That should fix the detachment issue.

@Zverik
Copy link
Contributor Author

Zverik commented May 28, 2024

Yes, that works, thank you Luka!

@JaffaKetchup JaffaKetchup changed the title [BUG] Map Controller shows an assert when in a ListView [BUG] External MapController lifecycle not synced with widget when destroyed and rebuilt May 30, 2024
@Zverik
Copy link
Contributor Author

Zverik commented Jun 5, 2024

Similar issue was reported on Discord, but with tabs instead of list items.

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Jun 9, 2024

I've run into a similar issue that cannot be resolved by keepAlive.
EDIT: Turns out this is because I was inadvertently fully rebuilding the map (moving it in and out of a subtree (disabling/enabling some wrappers) when a switch was flipped) - interestingly, the map loaded pretty much instantly - maybe this is the image cache?

@josxha
Copy link
Contributor

josxha commented Jun 9, 2024

@JaffaKetchup What is the expected behavior here?

At the moment a MapController is not meant to be a global map state and can only get attached to a map widget once. This behavior should be the same in v6 and in v7. This means that the MapController is only meant to be kept as long as the map widget exists.
A ListView does only keep widgets that are on the screen (+ maybe a buffer). This means that when a new map widget gets created, a new MapController should get used.
This makes me think that something prevented the map widget from being disposed in v6?

@Zverik
Copy link
Contributor Author

Zverik commented Jun 9, 2024

Note that a map controller can be created in a containing stateful widget, being a part of its state.

@JaffaKetchup
Copy link
Member

This means that when a new map widget gets created, a new MapController should get used.

As @Zverik said, this isn't always easy and/or possible. You'd need to create a layer to export the map controller on every build from the inherited state I guess.

Is there a reason why being able to reassign wouldn't work?

(Also moving this to P2 as the workaround seems to work fine).

@josxha
Copy link
Contributor

josxha commented Jun 10, 2024

Is there a reason why being able to reassign wouldn't work?

Not really, should be doable. It just wasn't designed that way in the past. Could be that it was a problem with the cyclic dependency of the map controller but that got removed in #1738.
I just can't tell much about potential side effects at the moment.

@FeofanGreek
Copy link

We have the same problem when using the global map controller.
But we are forced to do this because third-party application functions access the controller to obtain the current state or even simply convert coordinates into pixels.
The assignment is done by: return FlutterMap(
mapController: controller,
How can this situation be stopped?

@ibrierley
Copy link
Collaborator

I'd probably take a look at an Offstage widget to put the map in (and possibly move it offstage from the listview when outside the displayed widgets if it doesn't keep its state at that point still).

@josxha
Copy link
Contributor

josxha commented Jun 13, 2024

But we are forced to do this because third-party application functions access the controller to obtain the current state or even simply convert coordinates into pixels.

@FeofanGreek Could you give more information about the 3rd party application you are using?
When do you want to convert coordinates into pixels while the map widget does not exist?

@FeofanGreek
Copy link

I didn’t express myself correctly about the “Application”, of course we use functions inside our application, but launched in the background. However, the functions themselves require simple controller-based transformations. And it is not always possible to gain access through the instance of the screen on which the map is displayed.
Example: controller.camera.visibleBounds.west or controller.move(controller.camera.center, UserConfig.instance.data.startZoom) or controller.camera.pointToLatLng(Point(width, 0)) or controller.camera.latLngToScreenPoint(b )

But we are forced to do this because third-party application functions access the controller to obtain the current state or even simply convert coordinates into pixels.

@FeofanGreek Could you give more information about the 3rd party application you are using? When do you want to convert coordinates into pixels while the map widget does not exist?

@MobileDev500
Copy link

I have implemented the following function to recreate MapController and MapOptions each time FlutterMap is rerendered, but it still throws the error in described in OP:

MapController getMapController() {
    mapController?.dispose();
    mapController = MapController();
    return mapController!;
  }

  MapOptions getMapOptions() {
    return MapOptions(
        initialCenter: initialCenter,
        initialZoom: 15.0,
        minZoom: Random().nextDouble(),
  }

Even though minZoom field of MapOptions is randomized (eg. changes every time the function is called) Flutter still does not recognize that options have changed. How is this even possible?

@josxha
Copy link
Contributor

josxha commented Jun 13, 2024

You can try to use the pull request #1915 to see if works with it.

dependency_overrides:
  flutter_map:
    git:
        url: https://github.com/josxha/flutter_map
        ref: feat/assign-controller-to-map-widget-multiple-times

@FeofanGreek
Copy link

Hooray! Everything worked

@MobileDev500
Copy link

MobileDev500 commented Jun 13, 2024

The fix by @josxha works.

If anyone encounters the issue of Polygons not updating, the fix is to give new key value to PolygonLayer constructor whenever you need to update your Polygons.

@JaffaKetchup

This comment has been minimized.

@josxha

This comment has been minimized.

@JaffaKetchup

This comment has been minimized.

@JaffaKetchup
Copy link
Member

Please also double check https://docs.fleaflet.dev/usage/programmatic-interaction/external-custom-controllers#usage-within-a-state-system-model if using external state.
It may be that some of these issues occur when this is handled improperly. This won't apply to situations where the map controller is entirely with a stateful widget directly, but instead to solutions such as Riverpod/Provider.

@pogorv12
Copy link

pogorv12 commented Jun 26, 2024

I have the similar issue.

I have the stateful widget with the map and refresh buttom that suppose to bring map to initial position with tha mapController defined on the state widget level.

After updating 6.0.1 -> 7.0.1 I'm getting an error when that button is pressed and the setState initiated.

The following LateError was thrown building LayoutBuilder:
LateInitializationError: Field '_interactiveViewerState' has already been
initialized.

When the exception was thrown, the stack was:
http://localhost:52041/dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 296:3       throw_
http://localhost:52041/packages/flutter_map/src/map/controller/map_controller_impl.dart 19:40            set [_interactiveViewerState]
http://localhost:52041/packages/flutter_map/src/map/controller/map_controller_impl.dart 47:7             set interactiveViewerState
http://localhost:52041/packages/flutter_map/src/gestures/map_interactive_viewer.dart 103:12              initState

To eliminate the issue I've removed [final] from _interactiveViewerState declaration. This solved the error.

But, comparing to 6.0.1, 7.0.1 widget dont automaticaly catch the new initial LatLon during redrawing, so I have to manually add controller move after each location change

_mapController.move(LatLng(_location.latitude, _location.longitude), _mapController.camera.zoom);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error P: 2 (soon™?) S: core Scoped to the core flutter_map functionality
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants