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

Enable auto generation of ids for GeoJSON sources #7043

Merged
merged 2 commits into from Aug 6, 2018

Conversation

asheemmamoowala
Copy link
Contributor

Closes #6849.

Upgrades geojson related libraries to include auto-generation of ids for geojson features and clusters.

cc @ryanbaumann

Copy link
Member

@mourner mourner 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! Although worth noting that I just realized that Supercluster doesn't have an equivalent of generateId option, so if the original GeoJSON doesn't have id in features, they will only be present for the clusters, not the points. Should we add it there as well? Should be trivial.

@andrewharvey
Copy link
Collaborator

It looks like this is only done when you initialise a new GeoJSON source, if you subsequently use setData I assume generateId won't apply ids then? I think the behaviour should be made explicit somehow in the docs.

@asheemmamoowala
Copy link
Contributor Author

It looks like this is only done when you initialise a new GeoJSON source

@andrewharvey it should generate ids every time. workerOptions is built up in the constructor and is reused in calls to _updateWorkerData.

I just realized that this is somewhat of a problem though. id's are based on the index of elements in the features array, so they can change on subsequent calls to setData. This would mean any previously cached feature ids would point to a different feature after the new tiles are rendered. This would need documentation, but I also think that it is not a good API experience.

@andrewharvey
Copy link
Collaborator

I just realized that this is somewhat of a problem though. id's are based on the index of elements in the features array, so they can change on subsequent calls to setData. This would mean any previously cached feature ids would point to a different feature after the new tiles are rendered.

Previously cached internally to GL JS or by the user outside GL JS? I should be fine if ID's change since you're resetting the data anyway, and anyone who needs stable IDs can provide their own.

@asheemmamoowala
Copy link
Contributor Author

Supercluster doesn't have an equivalent of generateId option, so if the original GeoJSON doesn't have id in features, they will only be present for the clusters, not the points. Should we add it there as well?
@mourner -> mapbox/supercluster#97

@asheemmamoowala
Copy link
Contributor Author

Previously cached internally to GL JS or by the user outside GL JS?

@andrewharvey this would be both. setData needs to clear the corresponding feature state cache internally, and any feature ids cached or used externally will also need to be invalidated.

@hoogw
Copy link

hoogw commented May 14, 2019

How to use auto-generated-ID?
feature.id ?

@asheemmamoowala
Copy link
Contributor Author

@hoogw You can enable id generation by setting the generateId flag on your GeoJSON source.

@hoogw
Copy link

hoogw commented May 14, 2019

@asheemmamoowala
excellent! well done !

What if I have type: vector instead of type: geojson

I want to use map.setFeatureState(), have to have a unique id,
but the source from mapbox-tilesets ( or mapserver .... .pbf) do not have a id field, no unique value column.

How to get "generateid" ?

@hoogw
Copy link

hoogw commented May 14, 2019

this works fine.

            ` map.addSource("_geojson_source", {
          "type": "geojson",
          "data": ___GeoJson,
          
          
          // https://docs.mapbox.com/mapbox-gl-js/style-spec/#sources-geojson-generateId
          //https://github.com/mapbox/geojson-vt/pull/109
          //https://github.com/mapbox/mapbox-gl-js/pull/7043
          "generateId":true
          
          
          });`

"generateId":true also works in vector source?
If not, how to deal with vector source, to use map.setFeatureState() ???

                   `map.addSource("_tilesets_source",  {
                    type: 'vector',
                    url: 'mapbox://hoogw.aiuo72ew'

                     // ????
                       "generateId":true   // ??????
                    });`

and this ???

                 map.addSource("_tileserver_source",  {
                    type: 'vector',
                  
                    "tiles": ["http://localhost:10/tileserver/news-freedom-rank-2019/{z}/{x}/{y}.pbf"],
                    
                      // ????
                   "generateId":true,   // ??????

                    "minzoom": 0,
                    "maxzoom": 22
                    
                    });

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.

Automatically generate feature id values for geojson & cluster sources
4 participants