Preserve id property on internal GeoJSON objects

[mike-marcacci]

When a call to queryRenderedFeatures returns features from a GeoJSON source, the properties field of matching features is provided in the response, but their id field is not. Since this is the recommend place to store such an identifier (per the GeoJSON spec) it would be great if this were also returned in the results!

[mike-marcacci]

I went to implement this assuming it would be quite trivial (and it was!)... but then I discovered that the protobuf type set for id is uint64, which isn't flexible enough given that the GeoJSON spec states no preference and it's quite common to use UUIDs, etc.

[mike-marcacci]

It appears that the vector tile spec also has no preference of id type, but that it's quite established as an integer in the implementations. I submitted a quick PR to the spec to help clarify.

To get around this, I've whipped up PR #3211 which uses the index of the feature in the GeoJSON FeatureCollection as its id over the Protobuf layer, and maps it back in queryRenderedFeatures. This is probably NOT the cleanest way to handle it (the conditional logic in particular), but it provides a proof of concept.

[jfirebaugh]

Hi Mike, thanks for the bug report and PR!

the properties field of matching features is provided in the response, but their id field is not

I believe integer ids are expected to be preserved, as of mapbox/geojson-vt#70 and mapbox/vt-pbf#8 -- although it looks as if our package.json is compatible with, but does not strictly require, vt-pbf v2.1.0.

Preservation of non-integer ids is #2716.

[mike-marcacci]

Awesome! Thanks for linking #2716 - hadn't seen that one, and I definitely like the idea of its geobuf solution!

[mike-marcacci]

@jfirebaugh - also, just to be clear, this is easily fixed by adding this.id = feature.id here, but this will break existing apps where the GeoJSON id is a string, hence my workaround.

Alternatively, you could just add a check for type, and only set it if it's a number until #2716 lands.