-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require feature id in Map#setFeatureState #6974
Conversation
src/style/style.js
Outdated
@@ -794,6 +794,10 @@ 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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you pass an undefined value or empty string as the feature id? Is that considered valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
== null
works for undefined
and null
, but not for ""
. Ive added that additional check. Using ===
isn't appropriate here because 0
is a valid id value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good explanation
c342878
to
ce9a87c
Compare
does setFeatureState only works if the id-propertyname is "id"? whats happend if it is a different name? |
@strech345 the |
@asheemmamoowala |
mapbox/tippecanoe#615 adds the most basic |
Map#setFeatureState
should validate the input feature identifier object for theid
field, in addition tosource
andsourceLayer
.