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.