jerluc
commented
6 years ago @paulmach thanks for your time taking a look at this PR. It looks like I've run into an issue on a couple of different fronts:
As you pointed out, RFC-7946 clearly specifies that a feature's id can be either a number or a string. However, per the MVT spec and protobuf, the feature id must be an unsigned 64-bit integer. Within the MVT spec, it looks like there are several open issues (https://github.com/mapbox/vector-tile-spec/issues/94, https://github.com/mapbox/vector-tile-spec/pull/120, etc.) around updating their protobuf to match the GeoJSON RFC, which is expected to be supported in v3+. This is likely something that I'll have to work around until it is updated/resolved.
At the end of the day, however, it looks like orb's encoder circumvents today's type inconsistencies by avoiding encoding the feature id at all in the MVT format (see here). This is means that features always get encoded in MVT without their IDs (maybe this was intentional to avoid the type issues in the first place?).
In any case, feel free to close this PR if there's nothing else you can think of, as I can understand that this issue may be better resolved upstream in the MVT spec + protobuf before it would affect this repo.
paulmach
This hotfix allows feature IDs to be encoded and decoded properly, which is necessary to support functionality such as MapBox GL's
setFeatureState(...)which relies on the feature ID to uniquely identify the feature. This also helps to better conform to the MapBox Vector Tile specification.Note that previously, these values, though allowed in the
structs, were being omitted entirely in the encoder.