-
Notifications
You must be signed in to change notification settings - Fork 942
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
Fix crash on turf.intersect for touching polygons #2238
base: master
Are you sure you want to change the base?
Conversation
Seems sensible to me and is always good to cover edge (literally) cases. One for @rowanwins? |
Cool! Thx for merging, looking forward to the next release! |
Actually @pietervdvn this PR was not merged back to turf master, I did the opposite and merged latest turf master to this PR to update it and run the CI tests. Can you reopen it? |
oops! |
Thanks for this PR @pietervdvn So the main question mark here is what is the right output. Prior to v6 when we use So perhaps we should rejig this PR so it can once again provide that response? That would be a breaking change so we'd need to include it in the next major release. Extract from v5.1.5 jsdoc
Does that make sense? |
The old behaviour might be better, but I do not have the time nor experience to fix it. |
I've done some looking at this. |
npm test
at the sub modules where changes have occurred.npm run lint
to ensure code style at the turf module level.Submitting a new TurfJS Module: N/A
Hi,
While testing turfjs in MapComplete, I discovered that
turf.intersect(p0, p1)
would crash if these polygons shared a border over multiple points.I've added a testcase for this in the
turf-intersect
-package - which indeed crashed. Then I fixed it by checking that the result of the polygon-clipping would return a correct linear ring (at least for the outer ring).At last, I've updated the documentation to clarify that polygons that touch will return null as well.