turf-voronoi - retain properties from points

[maral]

Resolves #1450, voronoi now retains properties from the geoJson points as default, no option required.

• [ X ] Use a meaningful title for the pull request. Include the name of the package modified.
• [ X ] Have read How To Contribute.
• [ X ] Run npm test at the sub modules where changes have occurred.
• [ X ] Run npm run lint to ensure code style at the turf module level.

[maral]

Is this repo alive? What do I have to do to get the review?

[smallsaucepan]

Hi @maral. Yes the repo is still alive, and working toward a v7 release. Everyone here is doing their best so apologies we haven't gotten to your PR yet. No one wants to see your work go to waste. Will take a look for you and get this in when we can. Cheers

[maral]

Thanks for the response, appreciate it. I didn't want to be rude, but I kept checking on the PR every now and then, I guess I could have asked earlier :) Keep up the good work!

[smallsaucepan]

All good. Took a look at the PR and have an idea would like your thoughts on.

Rather than JSON stringify + parse to deep copy the props, what if we were to make cloneProperties() available outside turf-clone and use that? Might head off performance / memory problems if someone throws in a point with a million properties on it.

Also happy to submit this as is and revisit the performance if / when someone runs into a problem.

[maral]

Thanks! Of course, I would rather use cloneProperties, performance is key in this function and this would slow down (and will slow anyway, but less) also for all the people that don't need to retain the properties. I'll try to send the fix ASAP.

[maral]

Hi @smallsaucepan, I finally found time to finalize this and I am not sure what the best approach is here - in CONTRIBUTING, there is this little sentence:

Turf modules are small, containing a single exported function.

If I used cloneProperties, it would break this premise. Is it okay to just add the export keyword and all is good, or is there some convention how to do this properly?

[smallsaucepan]

Hi @maral. Keeping it to one function per module is generally true and been a worthwhile design goal. It's probably overkill though to separate the prop cloning into it's own module, especially as it's a clear subset of cloning a whole object. Making it a named export (as you have) is a good balance. Will review and try to get it merged.