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

Add support for custom properties in toFeatureCollectionJson method #116

Closed
wants to merge 2 commits into from

Conversation

MayGrass
Copy link

No description provided.

@MatanYadaev
Copy link
Owner

Hi @MayGrass, thanks for the PR. Can you share the motivation for this PR? Also, please add tests and update API.md

@MayGrass
Copy link
Author

Previously, when generating a GeoJSON feature collection, there was no straightforward way to include properties for each feature. This meant that after generating the GeoJSON, additional steps were required, such as decoding the JSON, iterating over the features, and adding the properties manually.

The updated test ensures that the toFeatureCollectionJson method correctly handles the provided properties and generates the expected GeoJSON feature collection.

@MatanYadaev
Copy link
Owner

I feel uncomfortable with the method signature you chose. The $properties parameter is an array of the parameters of each feature in the collection, which is a bit confusing and not an elegant API. What will happen if I enter more properties than the features? What if I want to have properties just on the fifth feature? etc
You can see Turf.js API for example https://turfjs.org/docs/#featureCollection
The properties are part of the features and not part of the collection.

var locationA = turf.point([-75.343, 39.984], {name: 'Location A'});
var locationB = turf.point([-75.833, 39.284], {name: 'Location B'});
var locationC = turf.point([-75.534, 39.123], {name: 'Location C'});

var collection = turf.featureCollection([
  locationA,
  locationB,
  location
]);

Anyway, for this package, I don't think it's important to push this type of change, as most of this package's value is about integrating with the DB.

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