Esri
Fixed: Request GeoJSON by default for feature services 1376
The following changes were made:
Updated the logic to default isModern to true for feature services.
Modified instances where isModern is used to ensure GeoJSON is requested by default.
@DoiMayank what issue is this addressing? If this is addressing a bug, can you please provide a replication case for the bug. Thanks!
@gavinr-maps This PR addresses the performance issues discussed in #1372 and #1373. The main problem comes from requesting features before confirming if the service supports GeoJSON. This causes unnecessary overhead because browsers have to convert the data to GeoJSON when the service could handle it directly.
The solution is to make isModern the default for feature services since GeoJSON output has been supported in ArcGIS Server starting with version 10.4, which was deprecated in February 2022. By doing this, we avoid the extra processing on the client side and take advantage of the service’s capability to return GeoJSON directly, improving overall performance.
@DoiMayank ok, thanks. So is the replication case in #1373 (copied below) the best test of this fix or something else?
@gavinr-maps I believe the solution proposed in https://github.com/Esri/esri-leaflet/issues/1376 is the best fix for this issue. I have implemented it in this PR, and @patrickarlt was also leaning in this direction.
Ok, thanks. I'll discuss with Patrick the best way to test this in the context of #1376.
Another interesting observation from this is that for services that do not support GeoJSON the object id field bust be OBJECTID or it likely wont work at all. This is because we don't wait for the metadata call on requests so a non-standard metadata field MIGHT be returned AFTER a query response comes back and we will get some strange behavior when converting to GeoJSON.
@DoiMayank I discussed this internally with the team. We all agreed that setting isModern to true bu default as described in https://github.com/Esri/esri-leaflet/pull/1403#pullrequestreview-2730535236 IF we also print a warning that service does not support GeoJSON and isModern is set to true the best place for this is probably in https://github.com/Esri/esri-leaflet/blob/master/src/Layers/FeatureLayer/FeatureManager.js#L78-L114