You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hello 👋
While using a Chart Library called Nivo (that uses d3-delaunay), we faced an issue with one of our Charts that had empty data.
Basically under the hood the code uses Delaunay.from with an empty array.
Using delaunay.find with an empty array returns NaN each time which kind of breaks the Chart Library workflow because it needed a number used as an index.
Repro
To reproduce the behavior you need to do the following steps :
import{Delaunay}from'd3-delaunay';constdelaunay=Delaunay.from([]);console.log(delaunay.find(0,0));// ⬅️ This returns NaN
Possible fix
Looking at the code I think I spotted where this can come from :
Basically in the case of points.length that is equal to 0, points.length >>> 1 is still equal to 0 so the operation returns NaN.
Also one other case that could happen is having the points.length equal to 1 but inedges[i] is equal to -1. With this case point.length >>> 1 will return 0 as well, so again trying to divide by 0 will return NaN.
So the fix would be to check those 2 cases separately to return -1 to match the other edge case you covered in the find function.
I can open a PR with a fix and the tests, I just wanted to make sure we want this fix or if the NaN case is actually something you wanted to keep 😄
The text was updated successfully, but these errors were encountered:
Nice find! We should definitely be consistent here — if other cases where the item can't be found return -1, all of them should, there's no special meaning to NaN in this case. PR would most definitely be welcome!
Hey @mourner ,
Thanks a lot for the quick answer and the insights!
I've created a PR to fix the issue, if you have the time to look at it that would be awesome 😄
Thanks a lot for what you are doing 🙏
Context
Hello 👋
While using a Chart Library called Nivo (that uses d3-delaunay), we faced an issue with one of our Charts that had empty data.
Basically under the hood the code uses
Delaunay.from
with an empty array.Using
delaunay.find
with an empty array returnsNaN
each time which kind of breaks the Chart Library workflow because it needed a number used as an index.Repro
To reproduce the behavior you need to do the following steps :
Possible fix
Looking at the code I think I spotted where this can come from :
d3-delaunay/src/delaunay.js
Line 140 in 7bf3fd0
Basically in the case of
points.length
that is equal to 0,points.length >>> 1
is still equal to 0 so the operation returnsNaN
.Also one other case that could happen is having the
points.length
equal to 1 butinedges[i]
is equal to-1
. With this casepoint.length >>> 1
will return 0 as well, so again trying to divide by 0 will returnNaN
.So the fix would be to check those 2 cases separately to return
-1
to match the other edge case you covered in thefind
function.I can open a PR with a fix and the tests, I just wanted to make sure we want this fix or if the
NaN
case is actually something you wanted to keep 😄The text was updated successfully, but these errors were encountered: