Skip to content
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

Revert Data Join example until feature.id type issues are resolved #6960

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

asheemmamoowala
Copy link
Contributor

#6580 updated the Data Join example to use the new Map#setFeatureState API. While the updated example shows the power of using the new API for data joins, it causes confusion with developers trying to use it in two ways:

  1. Input features must have numeric ids assigned. Non numeric/string ids (valid in GeoJSON) are not supported and break silently. ref: query*Features should preserve non-integer feature IDs #2716, Map#setFeatureState should reject feature identifier without an id field #6959
  2. Mapbox's own APIs sometimes discard the feature ids in input data when creating a Tileset.

cc @ryanbaumann @samgehret

@andrewharvey
Copy link
Collaborator

andrewharvey commented Jul 13, 2018

  1. Mapbox's own APIs sometimes discard the feature ids in input data when creating a Tileset.

ref #6019 (comment) but I wouldn't hold off the data join examples because of this.

@asheemmamoowala
Copy link
Contributor Author

@andrewharvey we have discovered a few other ways that the ids are re-created or dropped. Overall the demo creates more trouble for developers with data outside of their control. It should be back soon as a few PR fixes land.

A couple PRs related to this:

Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

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

Looks good!

@asheemmamoowala asheemmamoowala merged commit 0845880 into master Jul 17, 2018
@asheemmamoowala asheemmamoowala deleted the revert-data-join-example branch July 17, 2018 21:24
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.

3 participants