Closure bug with map click handler

Please see the notebook above. When I attach a click handler to the map, with a reference to a view in another cell, then change the value of the view, the click handler keeps the old value reference.

If I remove the “on load” wrapper, and just attach the click event handler to the map, then it adds the click handler every time the referenced view changes, resulting in multiple handlers on the map.

If I move the click handler initiation to the “on load” block in the map itself, the map refreshes every time the input changes (in practice, I want the map to zoom smoothly instead).

Is there a way around this? As it stands, I can’t figure out how to add a click handler to the map that references a view.

The cell that adds the load listener is running whenever the inputText changes, which is whenever you edit the text in the textarea.

{
  map.on('load', () =>
    map.on('click', evt => {
      console.log('clicked', inputText);
    })
  );
}

Each time it runs, it’s registering a new load event listener—in addition to all the previous ones. But since the map is already loaded, the new listener is never invoked, and so never registers another click event listener, and so only the first listener (which captured the original value of inputText) is ever invoked.

A couple’a ways you could fix this.

The first way would be to opt-out of reactivity and instead reference the textarea’s value (non-reactively) in your click event listener. That would look like this:

viewof map = {
  const container = html`<div style="height:350px;width:${width}">`;
  yield container; // Give the container dimensions.
  
  const map = container.value = new mapboxgl.Map({
    container,
    center: [0, 0],
    zoom: 0,
    style: 'mapbox://styles/mapbox/light-v9'
  });

  map.on('click', evt => {
    console.log('clicked', viewof inputText.value);
  });
  
  invalidation.then(() => map.remove());
}

(For that matter, you don’t even need to make a view for inputText, unless you want to reference it’s value reactively elsewhere in the notebook.)

Because the click listener is assigned within the map cell, you’ll never get more than one listener registered. And if the viewof inputText cell’s source code is edited, it’ll trigger the map cell to re-run and create a new map, but that should be fine.

The other way you could do it would be to register a new click event listener whenever inputText changes, but you must remove the old listener, too, using the invalidation promise. That would look like this:

{
  const clicked = evt => console.log('clicked', inputText);
  map.on('click', clicked);
  invalidation.then(() => map.off('click', clicked));
}
2 Likes

Thanks for the thorough explanation! Really helpful, and includes a couple of concepts I was missing (non-reactive reference, cell invalidation). I think the invalidation handler is exactly what I need - will give it a whirl.

1 Like

Glad to help! Let us know if we can be of further assistance.