Skip to content

Commit

Permalink
Parse string ids as ints
Browse files Browse the repository at this point in the history
  • Loading branch information
bfrengley authored and Asheem Mamoowala committed Aug 16, 2018
1 parent f449162 commit 136d42e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 9 deletions.
16 changes: 11 additions & 5 deletions src/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -787,11 +787,12 @@ class Style extends Evented {
return this.getLayer(layer).getPaintProperty(name);
}

setFeatureState(feature: { source: string; sourceLayer?: string; id: number; }, state: Object) {
setFeatureState(feature: { source: string; sourceLayer?: string; id: string | number; }, state: Object) {
this._checkLoaded();
const sourceId = feature.source;
const sourceLayer = feature.sourceLayer;
const sourceCache = this.sourceCaches[sourceId];
const featureId = parseInt(feature.id, 10);

if (sourceCache === undefined) {
this.fire(new ErrorEvent(new Error(`The source '${sourceId}' does not exist in the map's style.`)));
Expand All @@ -802,19 +803,20 @@ class Style extends Evented {
this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`)));
return;
}
if (feature.id == null || feature.id < 0) {
if (isNaN(featureId) || featureId < 0) {
this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided and non-negative.`)));
return;
}

sourceCache.setFeatureState(sourceLayer, feature.id, state);
sourceCache.setFeatureState(sourceLayer, featureId, state);
}

getFeatureState(feature: { source: string; sourceLayer?: string; id: number; }) {
getFeatureState(feature: { source: string; sourceLayer?: string; id: string | number; }) {
this._checkLoaded();
const sourceId = feature.source;
const sourceLayer = feature.sourceLayer;
const sourceCache = this.sourceCaches[sourceId];
const featureId = parseInt(feature.id, 10);

if (sourceCache === undefined) {
this.fire(new ErrorEvent(new Error(`The source '${sourceId}' does not exist in the map's style.`)));
Expand All @@ -825,8 +827,12 @@ class Style extends Evented {
this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`)));
return;
}
if (isNaN(featureId) || featureId < 0) {
this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided and non-negative.`)));
return;
}

return sourceCache.getFeatureState(sourceLayer, feature.id);
return sourceCache.getFeatureState(sourceLayer, featureId);
}

getTransition() {
Expand Down
8 changes: 4 additions & 4 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ class Map extends Camera {
*
* @param {Object} [feature] Feature identifier. Feature objects returned from
* {@link Map#queryRenderedFeatures} or event handlers can be used as feature identifiers.
* @param {number} [feature.id] Unique id of the feature.
* @param {string | number} [feature.id] Unique id of the feature.
* @param {string} [feature.source] The Id of the vector source or GeoJSON source for the feature.
* @param {string} [feature.sourceLayer] (optional) *For vector tile sources, the sourceLayer is
* required.*
Expand All @@ -1351,7 +1351,7 @@ class Map extends Camera {
* `map.getSource('some id').setData(..)` resets the cache of feature states and requires the
* caller re-apply the state as needed with the updated `id` values.
*/
setFeatureState(feature: { source: string; sourceLayer?: string; id: number; }, state: Object) {
setFeatureState(feature: { source: string; sourceLayer?: string; id: string | number; }, state: Object) {
this.style.setFeatureState(feature, state);
return this._update();
}
Expand All @@ -1364,11 +1364,11 @@ class Map extends Camera {
* @param {string} [feature.source] The Id of the vector source or GeoJSON source for the feature.
* @param {string} [feature.sourceLayer] (optional) *For vector tile sources, the sourceLayer is
* required.*
* @param {number} [feature.id] Unique id of the feature.
* @param {string | number} [feature.id] Unique id of the feature.
*
* @returns {Object} The state of the feature.
*/
getFeatureState(feature: { source: string; sourceLayer?: string; id: number; }): any {
getFeatureState(feature: { source: string; sourceLayer?: string; id: string | number; }): any {
return this.style.getFeatureState(feature);
}

Expand Down
38 changes: 38 additions & 0 deletions test/unit/ui/map.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,23 @@ test('Map', (t) => {
t.end();
});
});
t.test('parses feature id as an int', (t) => {
const map = createMap(t, {
style: {
"version": 8,
"sources": {
"geojson": createStyleSource()
},
"layers": []
}
});
map.on('load', () => {
map.setFeatureState({ source: 'geojson', id: '12345'}, {'hover': true});
const fState = map.getFeatureState({ source: 'geojson', id: 12345});
t.equal(fState.hover, true);
t.end();
});
});

t.test('resets state if source data is updated', (t) => {
const map = createMap(t, {
Expand Down Expand Up @@ -1397,6 +1414,27 @@ test('Map', (t) => {
map.setFeatureState({ source: 'vector', sourceLayer: "1", id: -1}, {'hover': true});
});
});
t.test('fires an error if id cannot be parsed as an int', (t) => {
const map = createMap(t, {
style: {
"version": 8,
"sources": {
"vector": {
"type": "vector",
"tiles": ["http://example.com/{z}/{x}/{y}.png"]
}
},
"layers": []
}
});
map.on('load', () => {
map.on('error', ({ error }) => {
t.match(error.message, /id/);
t.end();
});
map.setFeatureState({ source: 'vector', sourceLayer: "1", id: 'abc'}, {'hover': true});
});
});
t.end();
});

Expand Down

0 comments on commit 136d42e

Please sign in to comment.