feat(bun): add bun sqlite support

[Hebilicious]

Resolves #70

This PR adds support for bun-sqlite. This allows the bun:sqlite driver to be used with Bun when using the file protocol.
There is some difference in the implementation with better-sqlite3:

• BigInt support is different, it's not possible to retrieve a bigint from a query : [bun:sqlite] configurable javascript runtime type for sqlite integers oven-sh/bun#1536
• No lastInsertRowId support as there is no equivalent to the info returned by better-sqlite3 run
• Argument syntax is more strict and some combinations are not supported
• No native support for multiple line statements, therefore multiline statements will use batch under the hood.

Additionally, I did not implemement any of the server related tests, as I believe this driver should only be used for local databases.

There's one outstanding question regarding the bun lockfile which I did not include, and whether we should bother taking care of both lockfiles (This can be done automatically by the CI)

Before moving forward I would appreciate a review 🙏🏽

TODO :

• Run tests with CI
• Add documentation

[honzasp]

Thank you very much for you work! I will soon hand over this project to @penberg, so I will let him decide how to proceed. However, a few points regarding the PR:

1. The Bun code and the tests are a copy-paste of the better-sqlite3 code and Jest tests with a few modifications. I don't think that this is sustainable; the Bun and better-sqlite3 clients might be able to share some code, and we should be able to use the same test code in both Node and Bun.
2. If the Bun client cannot correctly implement intMode: "bigint" and executeMultiple(), I think it would be better to throw an error rather than producing wrong results or errors at runtime.

[Hebilicious]

Thanks for the review @honzasp, I will implement these changes.

• formatting : There is no shared prettier/eslint/editorconfig how should I format everything ?
• I left a few unresolved conversations, waiting for further feedback.
• Do you have any thoughts regarding the bun lockfile ? I think it's fine if we don't commit it.
• It's really cumbersome to have to duplicate all these tests, however in the future maybe it will be possible to unify them ?
• I've deployed this PR because I needed it : https://www.npmjs.com/package/@hebilicious/libsql-client, I can confirm it works (tested with drizzle + bun)
• Export map : I could be mistaken, but shouldn't import.edge-light point to http ?

[honzasp]

• formatting : There is no shared prettier/eslint/editorconfig how should I format everything ?

Yes, there is no automatic formatter. Format the code as you see fit, preferably in a way that's not too different from the existing code :)

• Do you have any thoughts regarding the bun lockfile ? I think it's fine if we don't commit it.

The npm lockfile is committed into the repo to make the testing in CI repeatable, so perhaps we should do the same for the Bun lockfile? (I have zero experience with Bun, but I assume that its lockfile works the same as the npm lockfile?)

• It's really cumbersome to have to duplicate all these tests, however in the future maybe it will be possible to unify them ?

Yes. Having two copy-pasted test suites wouldn't be acceptable for me. However, I will hand over this project to @penberg in a few days, and his opinion might be different :)

• Export map : I could be mistaken, but shouldn't import.edge-light point to http ?

edge-light is used by Vercel Edge Functions, which support both HTTP and WebSocket (both are tested in CI).

[Hebilicious]

Thanks for the review @honzasp. I resolved all of our conversations. Please let me know if I missed anything.

• formatting: I took the liberty of adding a prettier file that roughly mirror the existing configuration to make it easier to format stuff in this PR.

• lockfile: Yes the bun lockfile is similar to the npm one. Tbh as things are pretty new with bun, I am not aware of what the best practice would be in this scenario. I would assume that when running bun install in the ci, the package-lock.json file should be read and understood, and we do not need to maintain 2 lockfiles.

• I agree it's not ideal, I did it like this to avoid doing a major re-factor of the existing tests and to make sure bun:sqlite was working as intended. The bun:sqlite module has native code bindings that won't work with node, therefore we need to write the tests accordingly. Bun support for jest is still WIP (?), and to avoid issues and too much refactor in this PR, I went with the RY route. I'm interested in what @penberg has to say, but I think there's 3 options :

  1. keep the node and bun tests separated, but extract all common logic in a testing framework agnostic way.
  2. test everything with the bun test runner and the bun runtime
  3. test everything with jest (or equivalent) with both node and bun runtimes

  In the meantime this gives us confidence that the library is working. I personally think we should go with 1, as it will be more friendly to other runtimes such as Deno... Once a solution has been chose, I'm happy to do the refactor.

• edge-light: I can see that you updated the README now c6c9abd 👍🏽

To confirm that everything is working further, I deployed a fork of libsql-client-ts that I'm using with drizzle and bun https://www.npmjs.com/package/@hebilicious/libsql-client

import { drizzle } from "drizzle-orm/libsql"
import { createClient } from "@hebilicious/libsql-client"

const url = process.env.DATABASE_URL ?? "file:drizzle/local.db"
const authToken = process.env.DATABASE_AUTH_TOKEN

console.log(`Connecting to ${url}...`)
export const client = createClient({ url, authToken })
export const db = drizzle(client)

I have a few outstanding questions :

• Should I add CI tests in this PR ?
• Should I update the README with bun specific instructions ?

[penberg]

Hey @Hebilicious, please do add Bun to CI (otherwise we'll just end up breaking it) and also feel free to update the README.

[Hebilicious]

Hey @Hebilicious, please do add Bun to CI (otherwise we'll just end up breaking it) and also feel free to update the README.

Hi @penberg, I will update this.

Do you have any insight regarding the tests themselves ?

---

I agree it's not ideal, I did it like this to avoid doing a major re-factor of the existing tests and to make sure bun:sqlite was working as intended. The bun:sqlite module has native code bindings that won't work with node, therefore we need to write the tests accordingly. Bun support for jest is still WIP (?), and to avoid issues and too much refactor in this PR, I went with the RY route. I'm interested in what @penberg has to say, but I think there's 3 options :

• keep the node and bun tests separated, but extract all common logic in a testing framework agnostic way.
• test everything with the bun test runner and the bun runtime
• test everything with jest (or equivalent) with both node and bun runtimes

In the meantime this gives us confidence that the library is working. I personally think we should go with
1, as it will be more friendly to other runtimes such as Deno... Once a solution has been chose, I'm happy to do the refactor.

---

[penberg]

@Hebilicious I agree that we should keep Bun tests separate. Presumably in the future Bun gets compatible enough that the separate suite just becomes redundant and we'll switch to one suite.

[penberg]

Btw Vercel tests were using an expired token, which is sorted now. For Cloudflare, there seems to be some issue with the Hrana remote protocol, so I disabled the tests for now 37382a7

Both unrelated to this PR of course

[penberg]

@Hebilicious Btw, I switched from better-sqlite3 to a new libsql package today fb19cb7. And it seems to be working fine with Bun now that i tested it:

tursodatabase/libsql-js#10 (comment)

Do we still need to support bun:sqlite driver after the switch to libsql?

[Hebilicious]

@penberg That's great !

For my use case, I would personally still prefer to usebun:sqlite given the choice, as it's written natively for bun and therefore should be more performant.

On a side note, I am a little bit worried about the potential confusion between libsql-client-ts and libsql. Perhaps moving them to the same package/repo/monorepo would be a good idea 🤔 ?

Regarding the tests, I have a suggestion : Now that bettersqlite3 has been replace by libsql, how about we update the test suite to use the Bun test runner (instead of Jest), which would make it significantly easier to test the bun:sqlite driver, while also massively improving the CI speed ?