FeatureLayer _redraw method throws error if defined for a multilinestring

[mcalmus]

Describe the bug

If a FeatureLayer is created with FeatureLayerOptions that contains a pointToLayer method when one or more of the items in the layer is of type multilinestring, the _redraw method will throw an error when it tries to construct a LatLng object using an array of arrays.

The problem, here, is that instead of checking types and values, the _redraw method relies on the presence or absence of methods being defined to perform operations and then blindly sends what could be an invalid value to other places.

Reproduction

Something like this:

const options = {
    url: <someUrl>,
    options.pointToLayer = () => void
};
const layer = esri.featureLayer(options);
...

If a multilinestring happens to be one of the elements returned. It blows up from here:

esri-leaflet/src/Layers/FeatureLayer/FeatureLayer.js

Line 402 in 7ee4b61

latLng(geojson.geometry.coordinates[1], geojson.geometry.coordinates[0])

Logs

No response

System Info

leaflet 1.8.0
esri-leaflet 3.0.8

Additional Information

No response

[patrickarlt]

@mcalmus Can you give me a more detailed reproduction case here? pointToLayer is called when so that users can transform the x,y coordinates from the service it something that inherits from L.Layer (L.Marker, L.CircleMarker, ect...). As far as I know feature services don't support multiline strings as a geometry type and if they did we could convert it to a L.Polyline and skip pointToLayer.

I'm guessing your doing something like trying to convert a bunch of points to a multiline string on the fly which isn't really what this was intended to do but I can try to help if you provide a more detailed use case.

[mcalmus]

We are using pointToLayer to set a custom icon

  const options = {
    url: <someUrl>,
    pointToLayer: (feature, latlng) => new L.Marker(latlng, { icon: myCustomIcon })
};
const layer = esri.featureLayer(options);

There are several sets of layers available for the user to select from what is effectively a third-party service. Some of these layers have roads that include MultiLineString items. Because I know which of these layers are exclusively roads, I can selectively not include the pointToLayer definition for those layers, but it's nicer to just always include it in the configuration.

When it's included for one of these items, _redraw hits on

esri-leaflet/src/Layers/FeatureLayer/FeatureLayer.js

Line 399 in 7ee4b61

if (layer && layer.setStyle && this.options.pointToLayer) {

which should not be relevant.

[gavinr]

@mcalmus I'm having a hard time finding a public layer service that has those types of items - do you have the URL to a public service that we can use to create a replication case to see the issue? Thanks!

[patrickarlt]

Because I know which of these layers are exclusively roads, I can selectively not include the pointToLayer definition for those layers, but it's nicer to just always include it in the configuration.

@mcalmus so you want to be able to do this:

const layerOptions = {
  pointToLayer: (feature, latlng) => new L.Marker(latlng, { icon: myCustomIcon })
  style: { /*line and polygon style */}
}

const pointLayer = esri.featureLayer({
  url: urlToPointLayer,
  ...layerOptions
});

const roadLayer = esri.featureLayer({
  url: urlToLineLayer,
  ...layerOptions
});

And have it work? So we ignore pointToLayer for non-point layers?

[mcalmus]

@patrickarlt

Yes. That's the idea.

And have it work? So we ignore pointToLayer for non-point layers?

I'm not suggesting it should "ignore" pointToLayer so much as it shouldn't be using its presence as the way to identify the type of layer.

[mcalmus]

@mcalmus I'm having a hard time finding a public layer service that has those types of items - do you have the URL to a public service that we can use to create a replication case to see the issue? Thanks!

@gavinr I don't have a public layer. I'll see if I can get the data I'm using posted.

[martingraham]

Hi, sorry if resurrecting a 2.5 yr old issue is bad form but I'm hitting the same issue

and it's with one of the layers esri uses to demo the feature-layer:

https://developers.arcgis.com/esri-leaflet/api-reference/layers/feature-layer/

    var trails = L.esri
      .featureLayer({
        url: "https://services3.arcgis.com/GVgbJbqm8hXASVYi/arcgis/rest/services/Trails_Styled/FeatureServer/0"
      });

Objects returned as multilinestrings e.g.

geometry:
coordinates:
(3) [Array(1233), Array(572), Array(564)]
type: "MultiLineString"
id: 143
properties: {OBJECTID: 143, TRL_NAME: 'California Coastal Trail', TRL_ID: 10187, ELEV_MIN: 0, ELEV_MAX: 72, …}
type: "Feature"

[patrickarlt]

@martingraham thanks for jumping in. Can you post a full sample with the issue that you are seeing so I can take a look?

[martingraham]

Something like this? --> https://jsfiddle.net/bfm1v67j/

Comment out the pointToLayer property and the "Error: Invalid LatLng object:" errors stop (there
With it in the errors occur and not as many features are drawn either

I use the pointToLayer option as a standard option as @mcalmus did just to style marker icons when needed.
To tell whether it's needed or not per layer isn't really doable as it's for a tool for users which allows picking / choosing feature layers to show...

[patrickarlt]

Thanks @martingraham. I was able to figure out what was going on here. pointToLayer is intended to be called only for FeatureLayers containing points so it is used internally as kind of a check as to if the layer is a point layer or not. Since that layer is a polygon layer that is why you are seeing the error.

it's for a tool for users which allows picking / choosing feature layers to show

This isn't a great use case for Esri Leaflet since we don't support pulling in styles from the service so all your layers will get the same style. If that is your intent that is OK but the ArcGIS API for JavaScript would probably be better choice here since it will automatically load the styles for the layer.

That said I'll think about how to fix this. In the meantime you can determine the type of geometry ahead of time by looking at the geometryType in the metadata response to know what type of layer you are dealing with and set pointToLayer or style accordingly.

[martingraham]

Thanks @patrickarlt , yeh I can use the metadata functionality to undefine pointToLayer, cheers for the pointer :-)

[patrickarlt]

Glad that worked for you @martingraham. I still would like to leave this open since I think it is bad form to use the options you set to determine how to style the layer. We should call the approproate methods if set in options depending on the geometry type.