old markers are not removed when backing data changes

[gabrielgrant]

I'm generating <map.marker> instances by iterating over an array that is created (in a getter) based on model data. Elsewhere in the same template, I'm also looping over the same array in order to display card entries corresponding to the markers. When the model changes (with a <LinkTo>), the list of cards and the map both update to show the new entries. But the map also still has all the old entries (while the card list is only displaying the current data, as I'd expect). Logging the array confirms that the old entries are no longer present.

It certainly seems like the marker should be getting removed when the component is destroyed: https://github.com/kturney/ember-mapbox-gl/blob/master/addon/components/mapbox-gl-marker.js#L61

So it's very possible i'm just doing something silly, or that something outside ember-mapbox-gl's control that is causing the marker components to not be un-rendered, but having trouble figuring out what the difference is between these two {{#each}} loops, other than one being in a {{mapbox-gl}}

[kturney]

Hi @gabrielgrant, if you are able to share some code/code snippets, hopefully I can help you track down the issue.

[gabrielgrant]

Hey @kturney thanks for the quick reply, and sorry for taking a couple days to get back to this. Got a staging version up that shows the issue here: https://bb.gabrielgrant.ca/search-map/restaurants/around/Vancouver,%20BC,%20Canada

If you move the map it will pop up a button to redo the search centered on the new map center. Either clicking that (or just searching for a new location) will change the model.

It actually seems like the issue may be more subtle than i initially thought -- it looks like the first time the model changes, the pins are properly removed, but on the second (and subsequent) model changes, the old pins stick around.

The relevant template code is pretty straightforward, but including it here (with some irrelevant static HTML/SVG chunks removed) in case there's something silly I'm missing:

            {{#each this.paginatedResults as |r|}}
                <RestaurantCard
                  @restaurant={{r}}
                  {{on 'mouseenter' (fn this.setHovered r)}}
                  {{on 'mouseleave' (fn this.setHovered null)}}
                  class="{{if (eq r this.hovered) 'hovered'}}"
                />
            {{/each}}
        ...
        {{#mapbox-gl initOptions=(hash bounds=this.bounds) mapLoaded=(fn onMapLoad) as |map|}}
          <map.call @func='fitBounds' @args={{array this.bounds}} />
          <map.on @event='dragend' @action={{this.onBoundsChange}} />
          {{#if this.mapDefinedPlace}}
            <LinkTo @model={{this.mapDefinedPlace}} @query={{hash noScroll=true}} class="redo-search">
              Search in this area
            </LinkTo>
          {{/if}}
          <svg>
            ...
          </svg>
          {{#each this.paginatedResults as |r i|}}
            <map.marker @lngLat={{r.coords}} @initOptions={{hash anchor="bottom"}} as |marker|>
              <svg version="1.1" xmlns="http://www.w3.org/2000/svg"
              {{on 'mouseenter' (fn this.setHovered r)}}
              {{on 'mouseleave' (fn this.setHovered null)}}
              class="{{if (eq r this.hovered) 'hovered'}}"
              width="64" height="64" viewBox="-32 -32 64 64">
                ...
              </svg>
            </map.marker>
          {{/each}}
        {{/mapbox-gl}}
      </div><!-- end of .map -->

As for the controller, there's only a little bit of code related to the map display:

...
    //maps
    get bounds() {
        let vp = this.model.viewport;
        let sw = [vp.getSouthWest?.().lng() ||  vp.southwest.lng, vp.getSouthWest?.().lat() || vp.southwest.lat];
        let ne = [vp.getNorthEast?.().lng() ||  vp.northeast.lng, vp.getNorthEast?.().lat() || vp.northeast.lat];
        return [sw, ne]
    }

    @tracked hovered;
    @action setHovered(r) {
        console.log('setting hovered', r);
        this.hovered = r;
    }
    @action
    onMapLoad(map) {
        this.map = map;
    }
    @tracked mapDefinedPlace;
    @action
    onBoundsChange() {
        let bounds = this.map.getBounds();
        this.mapDefinedPlace = new Place({
            coords: bounds.getCenter(),
            viewport: {
                southwest: bounds.getSouthWest(),
                northeast: bounds.getNorthEast(),
            },
            accuracy: 'EXACT'
        });
        console.log('bounds changed', this.mapDefinedPlace);
    }
}

Other than that, paginatedResults is a getter whose content is based on the model (which is a representation of the user's location) and the parent route's model (the master list of locations). There's a not-so-small chunk of code there, but as I mentioned before, though, pretty sure that's working, since the card list is updated to properly reflect the latest location after a model change.

Certainly don't want to rope you into debugging my code for me, but happy to also just give you access to the whole project if you want/think that would help (it's private and on gitlab, though -- do you have an account there?)

Thanks again for looking into this, and let me know if there's anything else I can do to help you track down the issue

[gabrielgrant]

Also, just to make sure it doesn't cause confusion: you may notice an error showing up in the browser console that this.model.viewport.contains is not defined. This isn't actually an unhandled exception, though -- for some reason i just used console.error in lieu of console.log for that message 🤦 AFAICT there are no actual errors

[kturney]

Hi @gabrielgrant, I poked around with the ember inspector a bit, and I'm inclined to think something is going on with paginatedResults and ember's each.

I can see that there are only 20 restaurant-card elements in the DOM, but the ember inspector shows the same number of RestaurantCard and MapboxGlMarker components.

Some of the issue could depend on how the objects in the paginatedResults are created and the key (if any) used with each. This ember issue provides some explanation.

If I have time, and if you would like me to look through the code, my gitlab account is https://gitlab.com/turney.kyle

[gabrielgrant]

Hey @kturney thanks again for the reply, and sorry for again taking a while to get back to this.

Thanks for the link, that's definitely a bit of a weird gotcha that I hadn't come across before. That being said, AFAICT it doesn't seem to be applicable in this case: the problem there seems to be multiple items in the iterated array that have identical identity, causing Ember's to not replace DOM elements that should be re-rendered. In this case, though, the iterated array contains unique JS Objects[1], and ember is appending them to the DOM, but not removing the old ones.

I also don't think that issue would explain the weird behavior of it being the second change of the array elements triggering the problem.

That being said, I certainly see what you're saying about the number of card components matching the number of markers, despite not appearing in the DOM (should have noted this before, but I found this is a lot easier to observe in the Ember inspector by reducing the number of results by adding a QP to the URL: perPage=2)

So it certainly seems like this an Ember issue (or an issue with my usage thereof) rather than a problem in ember-mapbox-gl, so i think you're off the hook ;) In case you're curious, though, I've still invited you to the repo on gitlab (obv feel free to either decline the invite, or let me know if you want to be removed if you're getting annoying notifications or whatever).

Thanks again for this addon, and for your help here!

[1]: That the objects are different can be seen by looking at the actual values passed to the non-unrendered components -- they all have different Restaurant Name props, for example. FWIW, in addition to the contents of the results array being unique JS Objects, the results array itself is also unique per retrieval, since it's created by slicing the larger array within a getter:

    get paginatedResults() {
      return this.filteredRestaurants.slice(this.paginatedResultsStartIndex, this.paginatedResultsEndIndex);
    }

[gabrielgrant]

Filed an issue about this on the ember repo: emberjs/ember.js#19020