Migrate @turf/interpolate to TypeScript#2966
Conversation
| throw new Error("weight must be a number"); | ||
| } | ||
|
|
||
| box = box ?? bbox(points); |
There was a problem hiding this comment.
^ the defaults got pushed above, but the bbox(points) call has to be after the falsy check for points or we lose the useful error message from the validation.
| }); | ||
| // write interpolated value for each grid point | ||
| var newFeature = clone(gridFeature); | ||
| newFeature.properties ??= {}; |
There was a problem hiding this comment.
On the below line, the properties could have been null from the types, so we just make sure its there before proceeding.
packages/turf-interpolate/index.ts
Outdated
| * var addToMap = [grid]; | ||
| */ | ||
| function interpolate(points, cellSize, options) { | ||
| function interpolate<T extends "point" | Grid>( |
There was a problem hiding this comment.
What do you reckon about changing this to:
function interpolate<T extends Grid = "square">(
"point" is an element of the Grid union anyway, so no need to list it separately. This still seems to allow the conditional return type determination at the end of the function to work.
And I noticed if gridType is left empty, with the current definition the return type is Point | Polygon. However our runtime default is square which unambiguously infers Polygon. Setting that for the template default lets TS narrow the return type.
There was a problem hiding this comment.
Good catch that's a nice improvement
smallsaucepan
left a comment
smallsaucepan
left a comment
There was a problem hiding this comment.
Thank you @mfedderly. One more step towards TS throughout 🎉
2 participants
Benchmarks were mixed, but performance is pretty much the same before/after.