Inconsistent issue when removing a split map control from an ipyleaflet map

[lopezvoliver]

I am facing a very difficult issue to trace, so I came up with this almost reproducible example.

This animation shows the demo initially working as expected. Then, I refresh the app and there is an issue with the demo: the split-map control does not get removed correctly.

Setup

Let's start with defining a custom class inheriting from ipyleaflet.Map that can dynamically change between a split map and a "stack" map (layers stacked on top of each other).

import ipyleaflet
import traitlets
from traitlets import observe

class Map(ipyleaflet.Map,):
    map_type = traitlets.Unicode().tag(sync=True)

    @observe("map_type")
    def _on_map_type_change(self, change):
        if hasattr(self, "split_map_control"):
            if change.new=="stack":
                self.remove(self.split_map_control)  
                self.set_stack_mode()

            if change.new=="split":
                self.set_split_mode()     

    def set_stack_mode(self):
       self.layers = tuple([
           self.esri_layer,
           self.topo_layer
       ]) 

    def set_split_mode(self):
        self.layers = ()
        self.add(self.left_layer)
        self.add(self.right_layer)
        self.add(self.split_map_control)

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.osm = self.layers[0]

        esri_url=ipyleaflet.basemaps.Esri.WorldImagery.build_url()
        topo_url = ipyleaflet.basemaps.OpenTopoMap.build_url()

        self.left_layer = ipyleaflet.TileLayer(url = topo_url, name="left")
        self.right_layer = ipyleaflet.TileLayer(url = esri_url, name="right")
        self.topo_layer = ipyleaflet.TileLayer(url=topo_url, name="topo", opacity=0.25)
        self.esri_layer = ipyleaflet.TileLayer(url=esri_url, name="esri")

        self.stack_layers = [
            self.esri_layer,
            self.topo_layer,
        ]

        self.split_map_control = ipyleaflet.SplitMapControl(
            left_layer=self.left_layer, 
            right_layer=self.right_layer)


        if self.map_type=="split":
            self.set_split_mode()

        if self.map_type=="stack":
            self.set_stack_mode()

I haven't encountered the issue when testing the ipyleaflet code without solara.

Now let's add solara to the equation:

import solara
import ipyleaflet
import traitlets
from traitlets import observe
zoom=solara.reactive(4)
map_type = solara.reactive("stack")

class Map(ipyleaflet.Map,):
.... (same code defining Map as above)
....


@solara.component
def Page():
    with solara.ToggleButtonsSingle(value=map_type):
       solara.Button("Stack", icon_name="mdi-layers-triple", value="stack", text=True)
       solara.Button("Split", icon_name="mdi-arrow-split-vertical", value="split", text=True)   
    Map.element(
        zoom=zoom.value,
        on_zoom=zoom.set,
        map_type=map_type.value
    ) 

Page()

Could you please help diagnose this problem?

Here's a live version of an app where I am facing this issue (also inconsistently! try refreshing until you encounter the issue).

[iisakkirotko]

Hey @lopezvoliver! I looked into this, and it turned out to be quite complicated. The cause of the issue is two-fold:

• In https://github.com/quantstack/leaflet-splitmap, which ipyleaflet uses, if we remove SplitMapControl after we remove one of the left/right layers of the control, the removal function throws an error and doesn't finish executing. This is what causes the control to remain after having been removed. I look to fix this in fix: layer.getContainer could return null QuantStack/leaflet-splitmap#15.
• The more fundamental issue is in Solara - to make executing more efficient, we communicate updates in batches. However, this has the drawback of potentially mixing up the order of execution of the updates. What happens sometimes is that the update to map.layers is executed before that to map.controls, resulting in the above behaviour. However, this can't be fixed without introducing performance overhead into Solara, which we'd like to avoid.

There are some workarounds you could try:

• Do the update in two parts, based on two different properties / events, e.g. remove the control when map_type changes, and use an effect to then remove the layers.
• Give the map a key that changes when map_type is changed, like in this snippet. However, this will result in more rerenders of the map, and means that the __init__ function of the map has to restore all state to the map, instead of any of it being stored in the widget state.

[lopezvoliver]

Thanks @iisakkirotko for the insight

It took a while to examine this in detail but I finally managed to make a workaround using your first suggestion which I preferred to avoid having to restore the state of the map.

The workaround I ended up doing is what you suggested, but by doing a full round-trip between the map and the solara server by updating a solara reactive variable:

So when "removing" the control I set a dummy reactive variable to True:

                self.remove(self.split_map_control)  
                map_type_dummy.set(True)

which is also a trait of the Map:

class Map(ipyleaflet.Map,):
    map_type = traitlets.Unicode().tag(sync=True)
    map_type_dummy = traitlets.Bool(False).tag(sync=True)   # PATCH dummy trait 

    @observe("map_type_dummy")
    def _on_map_dummy(self, change):
        if change.new:
            map_type_dummy.set(False)
            self.set_stack_mode()             # HERE I update the layers, after a "round-trip" 

..... 


    Map.element(
        zoom=zoom.value,
        on_zoom=zoom.set,
        map_type=map_type.value,
        map_type_dummy = map_type_dummy.value
    ) 

Here's the full code

Interestingly, here's an attempt at the workaround which relies on a map_type_dummy trait but not using a solara reactive variable, which shows the same initial problem but gives interesting insight of the updates to the map by playing with a time.sleep command. Here's the code for this bad fix.

Looking forward to your fix to leaflet-splitmap getting merged too