Documentation improvement: make clearer that feature.id can be string, but only if string can be cast as int

[samfader]

Question

Our documentation on feature.id in setFeatureState and getFeatureState states that feature.id can be a string or a number. However, if you try a string that cannot be cast as an integer (i.e. "d-32-a"), you'll run into this error (The feature id parameter must be provided and non-negative).

A blog post on the Mapbox blog by @lobenichou and @asheemmamoowala confirms that The ids also have to be integers or strings cast as integers (e.g. “03989” or 3989).

I think it might be helpful to update the documentation to state that if you use a string, it must be valid when cast as an integer. If folks agree/I'm not missing anything, I'm happy to open up a PR that states this.

Links to related documentation

https://www.mapbox.com/mapbox-gl-js/api/#map#setfeaturestate

mapbox-gl-js/src/ui/map.js

Line 1393 in aab7551

* @param {string | number} feature.id Unique id of the feature.

https://www.mapbox.com/mapbox-gl-js/api/#map#getfeaturestate

mapbox-gl-js/src/ui/map.js

Line 1414 in aab7551

* @param {string | number} feature.id Unique id of the feature.

[rbrundritt]

This seems like an odd limitation of this feature. Would be good to see what is involved in getting any string value to work with this. In many data driven applications where features might be coming from an external GeoJSON feed, the developer won't have control over the ids on the features. The really odd thing is that setFeatureState works fine with any string.

[rbrundritt]

• Looking into this by looking at the getFeatureState function of the Style class there is code that parses the string as an int, then an if statement that verifies the feature id is not NaN and the id is not less than 0. (Note: 0 would work here). This then calls the getFeatureState function of the SourceCache class.
• The getFeatureState function of the SourceCache class, it passes the feature id number to the getState function of the SourceFeatureState Class.
• Looking at getState function in the SourceFeatureState class, the first thing it does is convert the id number into a string. Looking through all the code, it looks like this could definately be modified to support any string as an id and doesn't need to be a number. I'm not sure of the reason why this is currently designed to use an int, perhaps someone who is more familar on this code could investigate.

Pretty sure I found a bug: Looking at removeFeatureState function in the SourceFeatureState class, it does use the number feature id, but the if statements would fail if the id is 0. Lines 65, 70. Will need to test this to verify.

[asheemmamoowala]

I'm not sure of the reason why this is currently designed to use an int, perhaps someone who is more familiar on this code could investigate.

@rbrundritt There is a lengthy discussion at #2716

[andrewharvey]

was closed by #8550