Fixed: Request GeoJSON by default for feature services 1376

[DoiMayank]

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.

[gavinr-maps]

@DoiMayank what issue is this addressing? If this is addressing a bug, can you please provide a replication case for the bug. Thanks!

[DoiMayank]

@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.

[gavinr-maps]

@DoiMayank ok, thanks. So is the replication case in #1373 (copied below) the best test of this fix or something else?

[DoiMayank]

@gavinr-maps I believe the solution proposed in #1376 is the best fix for this issue. I have implemented it in this PR, and @patrickarlt was also leaning in this direction.

[gavinr-maps]

Ok, thanks. I'll discuss with Patrick the best way to test this in the context of #1376.

[patrickarlt]

@DoiMayank I think this might need some adjustments. In order to trigger the isModern behavior down in the service you'll need to set isModern to true when the service is created in https://github.com/Esri/esri-leaflet/blob/master/src/Layers/FeatureLayer/FeatureManager.js#L40. To do this you'll need to set isModern: true in the default options in https://github.com/Esri/esri-leaflet/blob/master/src/Layers/FeatureLayer/FeatureManager.js#L17-L28

After this change isModern will default to true everywhere and will never be undefined Which should simplify most of your remaining code because we no longer even need to check with the service if it supports GeoJSON or not. We assume it supports GeoJSON unless the user explictly tells us it does not.

If we want to resolve the potential performance issues in #1373 then we need to know ahead of time if the service supports GeoJSON which we can reasonably assume it does. Otherwise we have to wait for the metadata call before requesting any features which would be a major rewrite of FeatureManager.

[patrickarlt]

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.

[patrickarlt]

@DoiMayank I discussed this internally with the team. We all agreed that setting isModern to true bu default as described in #1403 (review) 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