Call to `geojson-polygon-self-intersections` sets `useSpatialIndex` to undefined

[ChrisLoer]

This call leaves the third argument undefined:

var selfIsectsData = isects(feature, function filterFn(isect, ring0, edge0, start0, end0, frac0, ring1, edge1, start1, end1, frac1, unique){
    return [isect, ring0, edge0, start0, end0, frac0, ring1, edge1, start1, end1, frac1, unique];
  });

But over in geojson-polygon-self-intersections there's backwards compatibility code that treats any non-object argument as a bare useSpatialIndex argument, so in this case the "undefined" value overwrites the default value of true.

https://github.com/mclaeysb/geojson-polygon-self-intersections/blob/aaa2fd26e9c457ab1b95fff52c28ceb914f2dcb3/index.js#L23-L29

I noticed this when I got unexpectedly slow performance using simplepolygon outside of turf's unkinkPolygon (which I think uses a slightly older fork of this code).

[mclaeysb]

Thanks for flagging this.

I think the logical fix is that geojson-polygon-self-intersections should check for options0 to be boolean before overwriting (rather then passing a boolean to it).

I've put it on my todo list for this month.

I knew there was interest from the Turf team and had been exchanging with @DenisCarriere in earlier issues) but I wasn't aware my code was used (as a copy) in Turf's unkinkPolygon (as plain copy, not as a package). What a nice thing to find out :)

@DenisCarriere would the Turf team be interested in this update? And why the choice for a plain copy and not a dependency?

[smallsaucepan]

Hi @mclaeysb. Found my way here looking into Turfjs/turf#2912

I'm not sure why your code was copied in to Turf, though maybe because it depends on Turf itself and someone didn't want to introduce circular dependencies? Though it would be cleaner for us, not to mention more respectful to you, if we just imported as a package.

Sounds like you're considering some new updates so should we start a discussion about the best way to package with Turf as well?

[mclaeysb]

I'm happy to consider doing more then a bugfix and also perform some structural updates to make it easier to import in Turf (e.g. make this e.g. an ESM package?) or consider other requirements.

I'm a freelance gis developper and if this work becomes considerable and is useful to a for-profit project (like Felt, @ChrisLoer ?) it would help me out to be sponsored.

[mclaeysb]

I have updated both packages to ESM modules written in TS, in v2.0.0 and v3.0.0 respectively, and while doing so also changed the API for geojson-polygon-self-intersections, with clearer options inputs.

@ChrisLoer Can you check if this fixes your problem?

@smallsaucepan This information so you can consider including this in your dependencies.

I only modernised the setup but didn't do a performance analysis or any other speed improvements. Those would require more time - to which I'd be open if this could be sponsored.

Cheers!

[ChrisLoer]

@ChrisLoer Can you check if this fixes your problem?

Yup -- I got hung up the first time I tried on the logging process dependency but looks like you just fixed that and now it seems to work well!