Bugfix - removeFeatureState fails with target.id === 0 (#8150) (#8164)

* Fix an edge case where 0 target.id would fail to clear the feature state * Check for number & string explicitly * Add tests for 0 feature ID * Fix logical check
mapbox · Apr 17, 2019 · 2d9001e · 2d9001e
1 parent 8af8aca
commit 2d9001e
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 2 deletions.
diff --git a/src/style/style.js b/src/style/style.js
@@ -878,12 +878,12 @@ class Style extends Evented {
             return;
         }
 
-        if (target.id && isNaN(featureId) || featureId < 0) {
+        if (target.id !== undefined && isNaN(featureId) || featureId < 0) {
             this.fire(new ErrorEvent(new Error(`The feature id parameter must be non-negative.`)));
             return;
         }
 
-        if (key && !target.id) {
+        if (key && (typeof target.id !== 'string' && typeof target.id !== 'number')) {
             this.fire(new ErrorEvent(new Error(`A feature id is requred to remove its specific state property.`)));
             return;
         }

diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js
@@ -1524,6 +1524,27 @@ test('Map', (t) => {
                 t.end();
             });
         });
+        t.test('remove properties for zero-based feature IDs.', (t) => {
+            const map = createMap(t, {
+                style: {
+                    "version": 8,
+                    "sources": {
+                        "geojson": createStyleSource()
+                    },
+                    "layers": []
+                }
+            });
+            map.on('load', () => {
+                map.setFeatureState({ source: 'geojson', id: 0}, {'hover': true, 'foo': true});
+                map.removeFeatureState({ source: 'geojson', id: 0});
+
+                const fState = map.getFeatureState({ source: 'geojson', id: 0});
+                t.equal(fState.hover, undefined);
+                t.equal(fState.foo, undefined);
+
+                t.end();
+            });
+        });
         t.test('other properties persist when removing specific property', (t) => {
             const map = createMap(t, {
                 style: {