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

Adding support to parsing single object to a Feature #23

Merged
merged 5 commits into from
Sep 18, 2016

Conversation

herkit
Copy link
Contributor

@herkit herkit commented Jul 13, 2016

I needed to be able to store Features in a database as single entities for later concatenation into a FeatureCollection, however this module forces Features into a FeatureCollection. I added a simple check if the data parameter is an array or not to allow conversion to a single non-collection wrapped Feature.

@caseycesari
Copy link
Owner

Thanks for the PR! I'll take a look at this soon.

});

addOptionals(geojson, settings);
if (util.isArray(objects)) {
Copy link
Owner

@caseycesari caseycesari Jul 14, 2016

Choose a reason for hiding this comment

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

Consider swapping this out for Array.isArray and dropping the require(util). requireing a module will break this library for vanilla JavaScript in the browser. Looking at the source of util.isArray, it's just a direct export of Array.isArray, so the code should be functionally the same.

@caseycesari
Copy link
Owner

I left a comment for your consideration. Thanks again for the PR!

@herkit
Copy link
Contributor Author

herkit commented Jul 25, 2016

Hi, I changed as per your comment, how Array.isArray missed my attention is beyond me.

@caseycesari
Copy link
Owner

Thanks again for the PR. Sorry about the delay to get this in. Merging now.

@caseycesari caseycesari merged commit 2122945 into caseycesari:master Sep 18, 2016
caseycesari pushed a commit that referenced this pull request Sep 18, 2016
caseycesari pushed a commit that referenced this pull request Sep 18, 2016
@caseycesari
Copy link
Owner

I just pushed v0.4.0 to npm, which includes this change. Thanks again.

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