Add Typescript definition

[DenisCarriere]

I've tried to keep the Typescript definition LOC to a minimum.

Added the following for utility (Optional & can be removed)

• test.types.ts
• npm scripts => types
• .gitignore

@mourner Looking forward to your feedback

I'll tackle geokdbush after the merge (will be very similar PR)

[mourner]

Is it really necessary to add this separate test.types.ts? Could the set up be simplified to e.g. run the existing test.js?

[DenisCarriere]

Come to think about it... unless you change your API, there's no real need to include the test files (I'll simply include the tests in the index.d.ts as a comment).

Including the test.ts comes from DefinitelyTyped (example with rbush), it's their way of testing if the types is properly working (however, it's not testing the actual code).

I'll drop it, it's really not necessary unless you publish a Major release (which I doubt will happen very frequently). Just @ me if you add new params or the output/input types change (or edit the index.d.ts directly.

[mourner]

@DenisCarriere I mean, what happens if you do tsc test.js? The tests run through all the code paths, so it should be equivalent in theory, except for a different file extension.

[DenisCarriere]

Running tsc test.ts only compiles the code into test.js "ES3 by default" (treat tsc like Babel except it uses the .ts file extension).

To compile & execute you would do this: tsc test.ts && node test.js

[DenisCarriere]

Here are the results of the compiled test.ts into ES3 using tsc

We could include the test Types as types.ts (instead of a comment or test.types.ts).

/types.ts

import * as kdbush from './'

// API
const points = [[110, 60], [130, 40]]
const index = kdbush(points)

// range
index.range(10, 10, 20, 20)
index.range(10, 10, 20, 20).map(id => points[id])

// within
index.within(10, 10, 5)
index.within(10, 10, 5).map(id => points[id])

// custom points
const xy = [{x: 110, y: 60}, {x: 130, y: 40}]
const latlng = [[60, 110], [40, 130]]
kdbush(xy, p => p.x, p => p.y)
kdbush(latlng, p => p[1], p => p[0])
kdbush(latlng, p => p[1], p => p[0], 64, Int32Array)

ES3 compiled tsc types.ts

"use strict";
exports.__esModule = true;
var kdbush = require("./");
// API
var points = [[110, 60], [130, 40]];
var index = kdbush(points);
// range
index.range(10, 10, 20, 20);
index.range(10, 10, 20, 20).map(function (id) { return points[id]; });
// within
index.within(10, 10, 5);
index.within(10, 10, 5).map(function (id) { return points[id]; });
// custom points
var xy = [{ x: 110, y: 60 }, { x: 130, y: 40 }];
var latlng = [[60, 110], [40, 130]];
kdbush(xy, function (p) { return p.x; }, function (p) { return p.y; });
kdbush(latlng, function (p) { return p[1]; }, function (p) { return p[0]; });
kdbush(latlng, function (p) { return p[1]; }, function (p) { return p[0]; }, 64, Int32Array);

[DenisCarriere]

Unless you want convert your test.js into Typescript? That would work... if we include tsc test.ts in the npm pretest.

[DenisCarriere]

what happens if you do tsc test.js

Looks like you can't compile without having .ts extension. Also you need to use the ES6 import for import * as kdbush from './' to properly test the correct Typescript paths/types.

$ tsc test.js
error TS6054: File 'test.js' has unsupported extension. The only supported extensi
ons are '.ts', '.tsx', '.d.ts'.

[DenisCarriere]

@mourner I believe the current state of this PR is the most "minimalistic" Typescript support without having to mix the test.js with Typescript tests. This way it doesn't force anyone to write the tests in Typescript and keeps the type definition testing isolated from the core testing.

[mourner]

Eh, such a shame that you can't properly add TypeScript support to a tiny module without a lot of cruft in the repo. :( Is using kdbush in a TypeScript project a big pain without these type definitions?

[DenisCarriere]

Is using kdbush in a TypeScript project a big pain without these type definitions

It's the biggest pain in the world if the Typescript definitions don't exist, it's up to the user to define them if they don't exists. Currently what people are doing is importing all the missing types from DT (ex: VS Code has 5 @types dependencies). Importing the @types definition for EVERY library that doesn't contain their own definition is such a pain.

If the definition doesn't exist, you can fallback to using require instead of import and unfortunately after that you lose all type checking since the library is now defined as any.

[DenisCarriere]

without a lot of cruft in the repo

We can drop the types.ts (Typescript tests), however the definition is fairly complex due to having a Class and the getX() & getY() methods.

If kdbush's definition was as simple as lets say @turf/bearing index.d.ts, then it would most certainly not require any Typescript testing since it's so simple, we could then be able to get away with simply adding index.d.ts & "types": "index.d.ts" in package.json.

Up to you if you decide to keep the types.ts or not, I'm good with keeping it or not.

[DenisCarriere]

lol... I know... it's a bit intense, ok here goes:

There's two different types of inputs (assumed & custom)

Assumed

This one is pretty straight forward and is automatically assigned by default

type Points = number[][]

Custom

The mythical <T> (can be defined as any letter or word), this mutates based on the user's input and that same type can be translated to other properties/methods/class.

interface KDBushStatic {
  (points: Points): KDBush<Points>
  <T>(points: T[], getX: (p: T) => void, getY: (p: T) => void, nodeSize?: number, ArrayType?: any): KDBush<T>
}

Lets look at this code above:

• User inputs custom points (not assumed type), then he is required to include getX() & getY() callbacks (types will fail if not provided).
• When the user inputs his custom Array of points, that same input type is then translated to getX() & getY() which enable the type checking based on custom entry, which makes it easier to define X & Y values.

declare class KDBush<T> {
  points: T[]
}

• The KDBush Class also has a <T> which allows you to translate the types given by the Array of points from the main method. This allows you to define the type of Array of points inside the class.

@mourner Let me know if this helps in any way, it's definitely a bit confusing a first (this only happens when the user is able to define his own input types).

Changes

Few minor changes (next commit), renamed the interface to KDBushStatic (no longer requires the <T>)

interface KDBushStatic {
  (points: Points): KDBush<Points>
  <T>(points: T[], getX: (p: T) => void, getY: (p: T) => void, nodeSize?: number, ArrayType?: any): KDBush<T>
}

declare const kdbush: KDBushStatic
declare namespace kdbush {}
export = kdbush

Here's an example of the benefits of having type checking with kdbush:

Valid

const xy = [{x: 110, y: 60}, {x: 130, y: 40}]
const index = kdbush(xy, p => p.x, p => p.y)
index.points[0].x
index.points[0].y

Not Valid

const xy = [{x: 110, y: 60}, {x: 130, y: 40}]
const index = kdbush(xy, p => p.foo, p => p.bar)
index.points[0].foo
index.points[0].bar

errors:

$ tsc types.ts
types.ts(23,33): error TS2339: Property 'foo' does not exist on type '{ x: number;
 y: number; }'.
types.ts(23,45): error TS2339: Property 'bar' does not exist on type '{ x: number;
 y: number; }'.
types.ts(24,17): error TS2339: Property 'foo' does not exist on type '{ x: number;
 y: number; }'.
types.ts(25,17): error TS2339: Property 'bar' does not exist on type '{ x: number;
 y: number; }'.

[mourner]

@DenisCarriere thanks a lot for the detailed explanation! I'll think this over a bit later, but the fact it gets so "intense" is exactly the reason why I'm hesitating to maintain types in the repo like this (as someone who does not use TypeScript) — if something is in the repo, I have to commit to maintaining it, and I can't maintain something I don't use or fully understand. So leaving it to a third-party to do in @types feels much safer.

[DenisCarriere]

Yea but I'm here! ;) I got your back for Typescript maintenance, lol

Well have a think about it, I don't mind publishing it to @types, the Microsoft folks are pretty helpful at providing feedback on Type definitions.

We can go that route, especially that geokdbush depends on kdbush (might get even messier 👎 ).

I'm 👍 closing this PR and I'll publish this definition to DT (95% of the work is already done, so this wasn't a waste).

[mourner]

@DenisCarriere thanks a lot! Sounds great.

[DenisCarriere]

@mourner Already getting some great feedback from the DT maintainers. https://github.com/DefinitelyTyped/DefinitelyTyped/pull/16107#pullrequestreview-34597565.