Fix bug when markers wouldn't update on state change (build)

[PaulMinguet]

When we create new markers or remove some while the map is open, the old markers wouldn't update and let the new appear or disappear. So I just reseted the markers as did in the initState (marker_cluster_layer.dart) in the build so for each state change the markers would reset.

[luca1994ioma]

please update the branch with this fix because is essential, thanks!

[luca1994ioma]

@lpongetti ping

[aytunch]

@PaulMinguet when I try your PR using this in my app:

  flutter_map_marker_cluster:
    git:
      url: https://github.com/PaulMinguet/flutter_map_marker_cluster.git
      ref: markers_update_on_change_state

I get a build error as follows. There are 2 initState and dispose overrides If I am not seeing it wrong.

[aytunch]

You might not have based from the latest master

I see some different names for function calls as

• _initializeAnimationController instead of _initializeAnimationControllers
• _initializeClusters instead of _initializeClusterManager
• There is no _mapCalculator = MapCalculator(widget.map);

[aytunch]

Also isn't it bad performance to call _initializeClusterManager and _addLayers inside of the build method?

[luca1994ioma]

any updates?

[PaulMinguet]

I'm on vacation, I'll take a look asap.

[TitanKing]

I'm on vacation, I'll take a look asap.

Hey @PaulMinguet thanks for letting us know. Appreciate your efforts man, really do appreciate this library.

[PaulMinguet]

Hey everyone, I pushed a commit with the latest version of the package (as I only edited marker_cluster_layer.dart there is no change in the other files). You can now try and use my commit with your projects. It should work with the latest features of the package. Let me now if something's not working well.

[aytunch]

Hi @PaulMinguet is adding

_initializeClusterManager();
_addLayers();

in to build the only way to solve this? Maybe instead we can use DidUpdateWidget and it might be more performant?

[PaulMinguet]

@aytunch yup, I think it's not that good for performances but I can't figure out why it doesn't update in the didUpdateWidget. I'm looking into it.

[PaulMinguet]

Actually the problem is that @lpongetti made a condition in the didUpdateWidget : oldWidget.options.markers != widget.options.markers. This condition is totally understandable, we don't want to update everything if the markers are the same. But the problem is that the oldWidget variable contains the new set of markers in it's oldWidget.options.markers and not the old (as said in the Flutter doc of the didUpdateWidget function. I don't know why it get the new ones. I'm still looking into it but if someone has the answer, I'm interested.

[aytunch]

@PaulMinguet

Actually when the markers change this should work fine. Can you try setting unique Keys for all the markers you create? Maybe it will help differentiate if old and new is different?

  @override
  void didUpdateWidget(MarkerClusterLayer oldWidget) {
    if (oldWidget.options.markers != widget.options.markers) {
      _initializeClusterManager();
      _addLayers();
    }
    super.didUpdateWidget(oldWidget);
  }

[PaulMinguet]

@aytunch I already tried it with my app, where all my marker have a key and it seems that oldWidget.options.marker and any other variable.options.marker have the exact same value. I tried to compare oldWidget.options.marker and widget.options.markers : they are the same array on widget update; I tried creating a new MarkerClusterLayer so it doesn't have the same markers but when I print them, I get the exact same array as oldWidget.options.markers. So I think it has to do with the Stream of MarkerClusterLayer maybe but I don't understand it all very well.

[melis-m]

Isn't didUpdateWidget just comparing the lists and not their content ?

I encountered this issue if I used a List for my markers that I would clear/add when updating markers, but if I just reassign to a new list each time my markers change, there's no issue. Which is understandable from the package POV, you don't really want to compare all the items of the list at each build, comparing only the list is faster.

So wouldn't this (reassigning marker lists instead of clear/add) be the "correct" way of doing this without having to call _initializeClusterManager() and _addLayers() at each build ?

I guess the missing thing then would be documentation, or a small disclaimer, or explanation, that developers using this package should reassign lists of markers and not update them in-place

[lpongetti]

Hi, sorry but my job and my son take up all my time.. I merged # 125 which may be inherent.. pls let me know if the problem persist

[github-actions[bot]]

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[Goden4you]

Isn't didUpdateWidget just comparing the lists and not their content ?

I encountered this issue if I used a List for my markers that I would clear/add when updating markers, but if I just reassign to a new list each time my markers change, there's no issue. Which is understandable from the package POV, you don't really want to compare all the items of the list at each build, comparing only the list is faster.

So wouldn't this (reassigning marker lists instead of clear/add) be the "correct" way of doing this without having to call _initializeClusterManager() and _addLayers() at each build ?

I guess the missing thing then would be documentation, or a small disclaimer, or explanation, that developers using this package should reassign lists of markers and not update them in-place

This is the real reason. Just create a new List of Markers and the layer will rebuild with actual information

[github-actions[bot]]

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.