Skip to content

Guard against non-numeric id in GeoJSON features - #4581

Merged
anandthakker merged 5 commits into
masterfrom
fix-4494
Apr 12, 2017
Merged

anandthakker merged 5 commits into
masterfrom
fix-4494

Conversation

@anandthakker

Copy link
Copy Markdown
Contributor

Closes #4494

Since we're using the vector tile binary format even for geojson-vt tiles, make
sure we conform to the MVT requirement that feature.id is an integer

@anandthakker
anandthakker requested a review from jfirebaugh April 12, 2017 14:03
@jfirebaugh

Copy link
Copy Markdown
Contributor

#4494 reports that the issue didn't occur in v0.32.1. Were you able to confirm that and determine why?

@anandthakker

Copy link
Copy Markdown
Contributor Author

@jfirebaugh yep - v0.32.1 is the last release before cf031a6, which is when the geojson-vt wrapper started copying feature.id.

this.properties = feature.tags;
if ('id' in feature) {
this.id = feature.id;
if ('id' in feature && !isNaN(feature.id)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment here explaining the original issue and the reason for !isNaN/parseInt?

@anandthakker
anandthakker merged commit a2dc4fa into master Apr 12, 2017
@anandthakker
anandthakker deleted the fix-4494 branch April 12, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants