Esri / esri-leaflet

A lightweight set of tools for working with ArcGIS services in Leaflet. :rocket:
https://developers.arcgis.com/esri-leaflet/
Apache License 2.0
1.61k stars 799 forks source link

Leaflet 1.9.3 error - can't access property "properties", layer.feature is undefined #1348

Closed PedroVenancio closed 1 year ago

PedroVenancio commented 1 year ago

Describe the bug

Hi, I'm testing Leaflet v1.9.3, updating from Leaflet v1.9.2 and I'm getting this error:

Uncaught TypeError: can't access property "properties", layer.feature is undefined

This happens when getting data from a service (L.esri.featureLayer) to construct the bindPopup or bindTooltip:

layer.bindPopup(function (layer) {
    var DateR = new Date(layer.feature.properties.Data_Recolha);
layer.bindTooltip(function (layer) {
    var DateR = new Date(layer.feature.properties.Data_Recolha);

The problem does not occur with Leaflet v1.9.2. In both cases, I'm using esri-leaflet-v3.0.9.

Reproduction

The issue can be seen in this sample:

https://developers.arcgis.com/esri-leaflet/samples/feature-layer-popups/

const earthquakes = L.esri
        .featureLayer({
          url: "https://sampleserver6.arcgisonline.com/arcgis/rest/services/Earthquakes_Since1970/MapServer/0"
        })
        .addTo(map);

      earthquakes.bindPopup(function (layer) {
        return L.Util.template(
          "<p>Earthquake <strong>{name}</strong> occured on {mo}/{dy}/{year_}.  It had a magnitude of {magnitude}.</p>",
          layer.feature.properties
        );
      });

Logs

Uncaught TypeError: can't access property "properties", layer.feature is undefined
    <anonymous> esri-leaflet-code-feature-layer-popups-f34906f913e34dc99d303746481f59bd.html:48
    _updateContent DivOverlay.js:277
    update DivOverlay.js:187
    onAdd DivOverlay.js:113
    onAdd Popup.js:135
    _layerAdd Layer.js:114
    whenReady Map.js:1477
    addLayer Layer.js:172
    openOn DivOverlay.js:63
    openOn Popup.js:131
    openPopup Popup.js:430
    _openPopup Popup.js:494
    fire Events.js:195
    _propagateEvent Events.js:311
    fire Events.js:204
    _fireDOMEvent Map.js:1452
    _handleDOMEvent Map.js:1401
    o DomEvent.js:108

System Info

Leaflet version: `v1.9.3`. ESRI Leaflet version: `v3.0.9`.

Additional Information

No response

gavinr commented 1 year ago

Hi, thank you for reporting the issue.

I see your issue. As you mentioned, it happens when using Leaflet v1.9.3. It seems like the feature property does not exist on the callback's event value when using Leaflet v1.9.3.

I reviewed the Leaflet release notes for v1.9.3 and do not see any breaking changes listed, so it's hard to tell if this is an issue with Leaflet or Esri Leaflet. This needs more investigation.

Reproduction steps

  1. Open https://jsbin.com/cesazol/3/edit?html,output
  2. Click popup:
    • Expected: popup opens
    • Actual: Error
  3. Open https://jsbin.com/cesazol/3/edit?html,output and switch the leaflet version to 1.9.2
    • Expected: popup opens
    • Actual: popup opens
gavinr commented 1 year ago

I think this issue is happening because of https://github.com/Leaflet/Leaflet/pull/8571. Specifically, these lines: https://github.com/Leaflet/Leaflet/pull/8571/files#diff-2b5b78f24babc48279b336b4e9d85623e3d1fd37c43fdfe5e7240e736d5fcf7aR425-R427 ... in Leaflet v1.9.3 the code was changed to:

Given the discussion around this change: https://github.com/Leaflet/Leaflet/pull/8571/files#r996012452, where it was just added to "fix" some unit tests, my initial thought is that this code in leaflet@1.9.3 is wrong and should be removed, but I'm not totally sure.

PedroVenancio commented 1 year ago

Hi @gavinr

Thanks for checking! Do you think this should be raised in Leaflet issue tracker?

patrickarlt commented 1 year ago

@gavinr @PedroVenancio I've opened https://github.com/Leaflet/Leaflet/issues/8761 to report the regression to Leaflet. I'm willing to bet we will need to fix this in Esri Leaflet since they made this change to fix a bug in Leaflet.

In the meantime 1.9.2 works and should be used.

gavinr commented 1 year ago

The fix for this was released in v3.0.10.

PedroVenancio commented 1 year ago

Thank you very much @gavinr @patrickarlt ! It's fixed in v3.0.10!

PedroVenancio commented 1 year ago

Hi @patrickarlt @gavinr

I was testing further and it seems that it's still something wrong when using clusters.

Please take a look at this sample: https://codepen.io/PedroNGV/pen/QWBOpJd

TypeError: can't access property "properties", layer.feature is undefined

Should I open a new issue, or maybe reopen this one?

Thank you very much!

gavinr commented 1 year ago

@patrickarlt do you think we need to apply the same change from PR #1350 into Esri Leaflet Cluster Feature Layer?

PedroVenancio commented 1 year ago

@patrickarlt do you think we need to apply the same change from PR #1350 into Esri Leaflet Cluster Feature Layer?

@gavinr @patrickarlt Do you think it is better to open a new issue, given it is related but it is not exactly the same issue?

gavinr commented 1 year ago

Yes, please log an issue in https://github.com/Esri/esri-leaflet-cluster