Bundle TypeScript type definitions with the package

[kevindigo]

Many Javascript libraries these days include TypeScript definitions. There are third-party better-sqlite3 TypeScript definitions available[1], but they are difficult to use. They aren't well documented, and some of the design choices seem ... odd. Since they are hosted within a general definitions github project, there is not a convenient place to ask questions or build a community. They are getting 5k downloads per week, so there is an audience for them.

It would be great to have official TypeScript support, and documentation.

[1] https://www.npmjs.com/package/@types/better-sqlite3

[StoneCypher]

Bump. This is extra important with the v4 migration

[millette]

After v8, @types... is still on v7.6.4.

[andreibadescu]

Support for typescript should be default

[jaens]

@JoshuaWise are you interested in a PR for this (now that TypeScript is very mainstream)?

There are several possibilities:

1. Put all the type definitions in sidecar *.d.ts files
2. Use JSDoc for declaring some types as comments (in the JS code):
   • which prevents the from going out of sync
   • enables type checking (though not much use in that, since the majority of the complexity is in the C++ side)
3. Use native Node.js TS support: Convert the .js to .ts and bump the engine requirement from v22 to v22.18

[m4heshd]

@jaens I hardly have any experience with TypeScript, but I managed to put together an index.d.ts file and have been shipping it with better-sqlite3-multiple-ciphers for a while now. Got no complaints. Definitely send a PR. Also take a look at m4heshd/better-sqlite3-multiple-ciphers#140 before you do.

[jaens]

@m4heshd Are you a maintainer of this repo? The question was for maintainers, since there's little point in making a PR if there's no interest in it, no? (especially given the state of open-source PRs lately...)

(and of course I'm aware of the existing typedefs, it's the first link in this issue... Your linked issue is typical ESM + TS fiddliness, which I'm used to (un)fortunately)

[m4heshd]

@jaens As you can see, I, in fact, am a maintainer.

[StoneCypher]

hold on i'm handling it. i did this years ago when i commented, but forgot to push it

doing it again so it's up to date

needs a regen pass and attw

[StoneCypher]

ok so there's a problem.

the right way to do this is to have the typescript compiler emit the types. i could hand-write a .d.ts that would be correct, but it would drift as the library was worked on. we can do this without adding typescript to the project through npx; that's not an issue.

what's an issue is that the Database class is implemented inside the factory, meaning that it doesn't get attached to the surface during resolution. Database isn't visible and there's no evidence that it'll be made visible, so it's dropped from the call surface. the thing that comes back from the factory, as such, is just a bare call surface; it doesn't even have new, let alone any of the necessary methods. fundamentally this is because it's prototypally assigned.

you get a clear error under ts 5.9; weirdly, ts7 just submarines this silently (i will raise this as an issue with them later.)

error TS9005: Declaration emit for this file requires using private name 'Thing'.  An explicit type annotation may unblock declaration emit.

it's a private name because Database is never nominated outside the closure, so there's no name to give to it. it's a subtle bug that has never mattered in the javascript because, well, why would you bother?

if it was typescript instead of javascript i could just declare it and we'd be done, but, it's not.

it was genuinely hard to solve this in a way that doesn't change call patterns and doesn't introduce extra subcalls (speed loss.) the call setup here is ... non-trivially odd.

i was able to get this as an export stub:

  declare const _exports: {
      (filenameGiven?: string, options?: {...}): import("./database") | undefined;
      new (filenameGiven?: string, options?: {...}): import("./database");
      withBinding: (...) => typeof import("./database");
      SqliteError: typeof import("./sqlite-error");  
  };
  export = _exports;

that seeeeeeeeems to do the trick.

i am opening two prs.

the first pr is a paranoid testing pr. it:

1. asserts a group of call patterns
2. asserts their correct implementation.

it is a safety rig, and exists just to show call pattern pre-extancy and correctness. it introduces no new non-testing behavior.

the second pr:

1. creates an npm script which keeps the .d.ts up to date, so that there is no risk of drift
2. builds the .d.ts with npx with typescript 5.9 because it has stronger error messages at this time (god damnit, ts7, you were supposed to be the chosen one)
3. emits and correctly places the .d.ts
4. asserts the .d.ts with attw ("are the types wrong," a ts-specific type linter)
5. relies on the behavioral tests from the first pr to show that nothing has changed from a caller's perspective

[StoneCypher]

test pr opened to show that all call patterns work

comparison table for upcoming pr added (in one instance it's a shorter call jump, which is a speed win; nothing breaks)

#1512

[StoneCypher]

okay after some merge drama, #1513 hosts a fix

consider to merge the tests in #1512 before #1513 for more safety, but they are not actually required