I'm a little bit concerned about just adding an epsilon here without really understanding the relationship between the cross product value and the accuracy in real terms. It looks like PostGIS actually takes a distance in meters instead of the epsilon or precision approach.

Are you speaking of ST_DWithin? I have very limited experience with PostGIS so I'm not sure what what the best approach would be here in terms of making the function signature here similar. I feel adding a parameter like this (precision/epsilon) gives the most control to the consumer of this function to do what they'd like and ideally you wouldn't even have to touch this parameter as it would Just Work™.

I think the only reason I even ran into this problem is because all of the test cases within this package were using simple integers instead of of lng/lat points.

Additionally I'm a little bit concerned that this change also does not correctly impact the ignoreEndVerticies option.

Yes I definitely skipped that bit of the function for my use case :P Let me explore that side a bit more and add some more test cases

Yeah I spent some time today looking into ST_DWithin and that's where I got the idea about using distance here. I think the problem with the epsilon/precision approach is that you're using it here on the cross product. I don't actually know what units the cross product is in, so it is hard to reason about the what that value means for the potential error allowed. Its also a little bit awkward to change the behavior of this method in a way that isn't exactly backwards compatible, even though I agree that this probably needs to work in the way you're suggesting.

Yeah I spent some time today looking into ST_DWithin and that's where I got the idea about using distance here.

So I found ST_DWithin when googling for a point-on-line solution but I think this turf module actually is meant to be closer to ST_Intersects, while ST_DWithin is probably closer to boolean-point-in-polygon. It seems ST_Intersects uses a constant 0.00001 meters for tolerance but both functions use meters as the default so is there any reason not to assume that for this function?

I think the problem with the epsilon/precision approach is that you're using it here on the cross product. I don't actually know what units the cross product is in, so it is hard to reason about the what that value means for the potential error allowed.

Its also a little bit awkward to change the behavior of this method in a way that isn't exactly backwards compatible, even though I agree that this probably needs to work in the way you're suggesting.

Yeah definitely. It feels as though one would have to sort of guess what value they need for their specific use case. As mentioned above, is there any reason not to assume meters for this? One way to go about this is to default the precision value to that of the input. For example, a point like [1,0] would work as the module does now where there's no room for error (cross === 0). But a point like [-75.2580499870244, 40.00180204907801] would use an epsilon value of 10^x where x is the number of decimal places. That being said, giving a default epsilon value for extra confidence would work in both cases and remove the need for converting to a string and counting decimal places.

In terms of backwards compatibility you're right in that this may "break" existing implementations. Originally I did have the undefined check to pass null instead of a default 10 but in order to keep backwards compatibility, I think it would be necessary to put that back in. I will also rename precision to tolerance to be closer to ST_Intersect. Having read more about floating points with this sort of check, I think epsilon is appropriate.

Other than that, I will make these changes now. I'm also open to other suggestions here as I think there's room for improvement and I don't want to have to keep using a custom fork 😛

Ok so I've opted for the more technical epsilon as a parameter. When dealing with lng/lat points, one will most likely have to use this parameter and pass in their own threshold value (ie { epsilon: 1e-10 }) in order to get the results they probably expect. I've also make this backwards compatible by not even considering it unless it is explicitly passed as an option. Added an additional test as well.