Remove sqlite3 vendoring

[benallfree]

Here you go @theogravity.

Closes #175

[theogravity]

How does it work if someone uses a non-sqlite3 library but conforms to the API?

The change means you must use the sqlite3 library.

[benallfree]

Doh I see. Ok let me work on this a little more, because you're right, /dist/index.d.ts is:

import * as sqlite3 from 'sqlite3';   // <-- This is what we don't want, right?
import { Statement } from './Statement';
import { Database } from './Database';
import { ISqlite, IMigrate } from './interfaces';
/**
 * Opens a database for manipulation. Most users will call this to get started.
 */
declare function open<Driver extends sqlite3.Database = sqlite3.Database, Stmt extends sqlite3.Statement = sqlite3.Statement>(config: ISqlite.Config): Promise<Database>;
export { open, Statement, Database, ISqlite, IMigrate };

[theogravity]

That's correct. You can try "import type" but i'm not sure if that will help or not

[benallfree]

Ok what about this...

I arrived here because I forked the sqlite3 project to extend its interface (TryGhost/node-sqlite3#1726). But because sqlite3 typings are vendored here, TS was stopping once it found the first definitions for sqlite3, which were in this package instead of my fork. This is a problem for anyone expecting TS to reference the actual sqlite3 typings.

It seems clear that the canonical sqlite3 definitions live with the sqlite3 package. If you don't want to add sqlite3 as a dep just to pull typings, there appears to be an independently maintained https://www.npmjs.com/package/@types/sqlite3 that stays updated.

When both a package's internal types and DefinitelyTyped (@types/package) types are present, TypeScript prefers the package's internal types.

So if you used @types/sqlite3 instead of vendoring it, I think it would cover every case:

1. People using sqlite3 and expecting the typings shipped with it (most common?)
2. People using sqlite3-compatible providers
3. People using experimental forks of sqlite3

@types/sqlite3 seems like a win to me.

I haven't tested it, but every sqlite3 user who wanted the "real" typings could do this as a workaround in the mean time:

// tsconfig.json

{
  "compilerOptions": {
    "paths": {
      "sqlite3": ["node_modules/sqlite3"]
    }
  },
  "exclude": ["node_modules/node-sqlite/dist/vendor-typings/sqlite3"]
}

I think it's rare that the sqlite3 interface will change, but it can happen. My personal feeling is that it's not totally accurate to vendor dependencies and then claim zero dependencies. This is definitely a case that makes some sense, but since this package doesn't own the sqlite3 interface definition, my humble view is that it really should be treated as a dependency in some way. Perhaps the sqlite3 team could be encouraged to take over @types/sqlite3 for the exact reason that the sqlite3 interface is a de-facto standard used by others not using the sqlite3 package. I can definitely understand not making everyone pull in the kitchen sink just for some typings.

What do you think about adding @types/sqlite3 as a dep?

[theogravity]

I think the problem with @types/sqlite3 was that it was missing a method at the time, specifically loadExtension, so I had to copy it over and add it in manually. I guess in a way, I had the same issue you did and did that to work around it. If it was present, then it could def be a dependency.

Looking at the current version, it still is missing it

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/sqlite3/index.d.ts

[benallfree]

Dang it... ok, then what about this... :)

It seems like node-sqlite + sqlite3 is the most common configuration since the docs even open with the suggestion to install both. What if we assume this is the most common use case and remove the vendoring? And update the docs with instructions for using a alternative providers with Typescript support by adding "paths": { "sqlite3": "node_modules/node-sqlite/dist/vendor-typings/sqlite3" } to their tsconfig.

[theogravity]

It may be the common config, but I'm not sure what percentage out there uses a non-common one. I'd rather not make the DX worse.

Could you try sending a PR to update the definitelyTyped definitions with the ones from the sqlite3 project? If you can do that, I can make it a dependency.

If not that, then forking this repo to make the changes you need is probably the way to go. You can reference your git repo in the package.json deps in your project to bypass publishing on npm I think.

Other than that, I'm not willing to do anything that would upset the DX around this library.