-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(core): replace better-sqlite3 with node:sqlite in the sqlite adapter #2106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
gruntlord5
wants to merge
2
commits into
emdash-cms:main
Choose a base branch
from
gruntlord5:node-sqlite-adapter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| "emdash": minor | ||
| --- | ||
|
|
||
| Updates the `sqlite()` adapter and CLI to use Node's built-in `node:sqlite` driver instead of better-sqlite3. SQLite sites no longer depend on a natively compiled binary, which removes `NODE_MODULE_VERSION` rebuild errors after Node upgrades and glibc incompatibilities on shared hosting. | ||
|
|
||
| Requires Node.js 22.15 or later. If you are on an older Node 22 release, upgrade Node before updating. | ||
|
|
||
| Connection pragmas are now applied wherever the package opens a SQLite database, so sites using the runtime `sqlite()` adapter get the same settings the CLI already applied: `journal_mode = WAL` (readers no longer block on the writer, and FTS5 shadow tables survive a mid-write process kill), `busy_timeout = 5000` (a competing writer waits instead of failing with `SQLITE_BUSY`), and `foreign_keys = ON`. | ||
|
|
||
| Query parameters are normalized so binding behaviour matches the old driver: `undefined` binds as `NULL` (accepted by better-sqlite3, rejected by `node:sqlite`), and a `Date` raises an actionable error instead of silently binding `NULL`. Booleans now bind as `0`/`1` — neither driver accepted them before, so boolean filters that previously threw at bind time (for example the plugin storage query API, whose `WhereValue` type has always advertised `boolean`) now work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| /** | ||
| * better-sqlite3-compatible wrapper around node:sqlite. | ||
| * | ||
| * Kysely's built-in SqliteDialect drives the database through better-sqlite3's | ||
| * surface: `prepare()`, statement `.reader` / `.all(params)` / `.run(params)` / | ||
| * `.iterate(params)`, and `close()`. node:sqlite's DatabaseSync supports the | ||
| * same operations but binds parameters as spread arguments and has no | ||
| * `.reader` flag. This wrapper bridges the two so the dialect needs no native | ||
| * compiled dependency. | ||
| * | ||
| * `.reader` is derived from `StatementSync.columns()` (column count > 0), | ||
| * which — like better-sqlite3's flag — is true for any statement that returns | ||
| * rows, including `INSERT ... RETURNING`. | ||
| * | ||
| * Requires Node >= 22.15 (node:sqlite unflagged + StatementSync.columns()). | ||
| */ | ||
|
|
||
| import { DatabaseSync } from "node:sqlite"; | ||
|
|
||
| /** | ||
| * The subset of better-sqlite3's Database API that emdash consumes: what | ||
| * Kysely's SqliteDialect calls, plus `exec`/`pragma` used at connection setup. | ||
| */ | ||
| export interface NodeSqliteCompatDatabase { | ||
| close(): void; | ||
| prepare(sql: string): NodeSqliteCompatStatement; | ||
| exec(sql: string): void; | ||
| pragma(pragma: string): void; | ||
| } | ||
|
|
||
| export interface NodeSqliteCompatStatement { | ||
| readonly reader: boolean; | ||
| all(parameters: ReadonlyArray<unknown>): unknown[]; | ||
| run(parameters: ReadonlyArray<unknown>): { | ||
| changes: number | bigint; | ||
| lastInsertRowid: number | bigint; | ||
| }; | ||
| iterate(parameters: ReadonlyArray<unknown>): IterableIterator<unknown>; | ||
| } | ||
|
|
||
| /** | ||
| * Open a SQLite database via node:sqlite, exposed through the | ||
| * better-sqlite3-compatible surface above. Pass the result directly to | ||
| * Kysely's `new SqliteDialect({ database })`. | ||
| */ | ||
| export function openNodeSqliteDatabase(path: string): NodeSqliteCompatDatabase { | ||
| const db = new DatabaseSync(path); | ||
|
|
||
| // Connection defaults, applied here because this is now the single place the | ||
| // package opens a SQLite database — previously only the CLI path | ||
| // (database/connection.ts) set them, so sites running through the runtime | ||
| // adapter (db/sqlite.ts) silently got neither. | ||
| // | ||
| // WAL: readers don't block on the writer, and writes land in a log before | ||
| // being applied — this is what prevents FTS5 shadow-table corruption if the | ||
| // process is killed mid-write. No-op for `:memory:`. | ||
| db.exec("PRAGMA journal_mode = WAL"); | ||
| // Wait for a competing writer instead of failing the query outright; without | ||
| // it a concurrent write (backup, second process) surfaces as SQLITE_BUSY. | ||
| db.exec("PRAGMA busy_timeout = 5000"); | ||
| // Referential integrity is off by default in SQLite; the schema declares | ||
| // foreign keys, so enforce them. | ||
| db.exec("PRAGMA foreign_keys = ON"); | ||
|
|
||
| return { | ||
| close: () => db.close(), | ||
| exec: (sql) => db.exec(sql), | ||
| pragma: (pragma) => db.exec(`PRAGMA ${pragma}`), | ||
| prepare(sql) { | ||
| const stmt = db.prepare(sql); | ||
| return { | ||
| reader: stmt.columns().length > 0, | ||
| all: (parameters) => stmt.all(...toBindings(parameters)), | ||
| run: (parameters) => stmt.run(...toBindings(parameters)), | ||
| iterate: (parameters) => stmt.iterate(...toBindings(parameters)), | ||
| }; | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** Parameter type accepted by node:sqlite statement bindings. */ | ||
| type SQLInputValue = null | number | bigint | string | Uint8Array; | ||
|
|
||
| /** | ||
| * Normalize Kysely's compiled parameters to what node:sqlite accepts. | ||
| * | ||
| * node:sqlite binds a narrower set of JS types than better-sqlite3 did, and | ||
| * differs in both directions, so the values are mapped rather than cast: | ||
| * | ||
| * - `boolean` — rejected by BOTH drivers (better-sqlite3: "SQLite3 can only | ||
| * bind numbers, strings, bigints, buffers, and null"). SQLite has no boolean | ||
| * type and stores them as 0/1, and `json_extract` yields 0/1 for JSON | ||
| * booleans, so mapping here makes boolean filters work rather than throw — | ||
| * e.g. the plugin storage query API, whose `WhereValue` advertises `boolean` | ||
| * (see plugins/types.ts) but which threw at bind time on either driver. | ||
| * - `undefined` — better-sqlite3 bound it as NULL; node:sqlite throws. Kysely | ||
| * passes it straight through (e.g. `.where(col, "=", undefined)` from an | ||
| * unset optional filter), so mapping to null preserves the old behaviour | ||
| * instead of turning a previously-working query into a runtime TypeError. | ||
| * - `Date` — better-sqlite3 threw; node:sqlite silently binds NULL, which | ||
| * would turn a loud programming error into silent data loss. Rethrow with | ||
| * an actionable message: emdash stores timestamps as ISO strings. | ||
| * | ||
| * Everything else (number, bigint, string, Uint8Array/Buffer, null) binds | ||
| * identically on both drivers and is passed through untouched; any remaining | ||
| * unsupported type is still rejected by node:sqlite at bind time. | ||
| */ | ||
| function toBindings(parameters: ReadonlyArray<unknown>): SQLInputValue[] { | ||
| return parameters.map((value) => { | ||
| if (typeof value === "boolean") return value ? 1 : 0; | ||
| if (value === undefined) return null; | ||
| if (value instanceof Date) { | ||
| throw new TypeError( | ||
| "Cannot bind a Date to a SQLite parameter; convert it to an ISO string first (e.g. date.toISOString()).", | ||
| ); | ||
| } | ||
| // eslint-disable-next-line typescript/no-unsafe-type-assertion -- Kysely hands through the values it compiled into the query; node:sqlite rejects any remaining unsupported type at bind time, same as better-sqlite3 did | ||
| return value as SQLInputValue; | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[needs fixing]
toBindingscasts Kysely's parameters toSQLInputValue[]without any value mapping, butSQLInputValueexcludesboolean.better-sqlite3accepts JavaScript booleans and stores them as0/1;node:sqliterejects booleans at bind time, so any query that inserts, updates, or selects on a boolean value will throw a runtime TypeError. This is a real compatibility gap for any code that relied on the old driver's coercion.Map booleans to integers before spreading them into the statement: