feat: migrate SQLite connector to built-in node:sqlite (#317)#330
Merged
Conversation
Replace better-sqlite3 with Node's built-in node:sqlite module, eliminating
native C++ compilation (node-gyp) and prebuild friction. SQLite now needs no
driver package and is always available.
API mapping:
- new Database(path, {readonly}) -> new DatabaseSync(path, {readOnly})
- db.defaultSafeIntegers(true) -> stmt.setReadBigInts(true) (per-statement,
centralized in a private prepare() helper)
- db.inTransaction -> db.isTransaction
Behavior preserved vs better-sqlite3:
- run() returns BigInt `changes` with setReadBigInts; normalized via Number()
- foreign keys default off (enableForeignKeyConstraints: false)
Build/runtime gotchas fixed:
- tsup removeNodeProtocol stripped `node:sqlite` -> `sqlite` (unresolvable);
set removeNodeProtocol: false
- node:sqlite emits ExperimentalWarning at load time; install a targeted
warning filter (suppress-experimental-warning.ts) before loading node:sqlite,
and load it via a non-hoisted lazy dynamic import() inside connect()
Other:
- drop better-sqlite3 / @types/better-sqlite3; bump engines.node to >=22.5.0
- CI run-tests Node 20 -> 22; update install/quickstart docs
- remove better-sqlite3 native-build approvals from pnpm-workspace.yaml
BREAKING CHANGE: requires Node.js >= 22.5.0
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates DBHub’s SQLite connector from better-sqlite3 to Node.js’s built-in node:sqlite (Node >= 22.5.0), removing native compilation friction while preserving existing connector behavior (including BigInt reads and default foreign-key behavior).
Changes:
- Replace
better-sqlite3usage withnode:sqlite(DatabaseSync+ per-statementsetReadBigInts(true)), and add a targeted suppression for Node’s one-timeExperimentalWarning. - Fix bundling/runtime loading details (tsup
removeNodeProtocol: false, lazyawait import("node:sqlite")). - Raise the Node engine floor and update CI + docs; remove
better-sqlite3dependencies and pnpm native-build approvals.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tsup.config.ts |
Prevents tsup from stripping the node: protocol so node:sqlite remains resolvable post-bundle. |
src/index.ts |
Updates SQLite connector “driver” identifier from better-sqlite3 to node:sqlite. |
src/connectors/sqlite/suppress-experimental-warning.ts |
Adds a targeted process warning filter to suppress the node:sqlite ExperimentalWarning. |
src/connectors/sqlite/index.ts |
Migrates connector implementation to node:sqlite, adds prepare helper to enable BigInt reads, lazy-loads module, and preserves FK default behavior. |
src/connectors/__tests__/sqlite.integration.test.ts |
Removes the unused better-sqlite3 import from SQLite integration tests. |
pnpm-workspace.yaml |
Removes better-sqlite3 native-build approvals/only-built settings. |
pnpm-lock.yaml |
Removes better-sqlite3 and its transitive native-install dependencies from the lockfile. |
package.json |
Raises Node engines to >=22.5.0, removes better-sqlite3 + types, and keeps other drivers optional. |
docs/quickstart.mdx |
Updates prerequisite Node version guidance. |
docs/installation.mdx |
Documents SQLite as built-in (node:sqlite) and updates driver/--demo notes accordingly. |
.github/workflows/run-tests.yml |
Updates CI Node major from 20 to 22 for unit + integration jobs. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comment on lines
389
to
391
| const quotedTableName = quoteIdentifier(tableName, "sqlite"); | ||
| const rows = this.db.prepare(`PRAGMA table_info(${quotedTableName})`).all() as SQLiteTableInfo[]; | ||
| const rows = this.prepare(`PRAGMA table_info(${quotedTableName})`).all() as unknown as SQLiteTableInfo[]; | ||
|
|
Member
Author
There was a problem hiding this comment.
Good catch — this was real. It's already fixed in 5787fb1 (the follow-up cleanup commit): getTableSchema now goes through a new queryAll<T> helper that uses a plain this.db.prepare() without setReadBigInts, so notnull/pk come back as numbers and notnull === 1 compares correctly. Introspection (small integer flags) stays on plain numbers; only executeSQL uses setReadBigInts for 64-bit user-data precision.
Comment on lines
26
to
30
| - name: Setup Node.js | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: '20' | ||
| node-version: '22' | ||
| cache: 'pnpm' |
Member
Author
There was a problem hiding this comment.
actions/setup-node resolves a bare major like '22' to the latest available 22.x release, never an early patch — so it can't land on a pre-22.5 runner that lacks node:sqlite. Pinning to '22.5.0' would actually be counterproductive: it forces CI onto the oldest supported patch, where node:sqlite is at its most experimental. Keeping '22' (newest 22.x, always ≥ 22.5) is the safer choice, so leaving this as-is.
Comment on lines
71
to
75
| - name: Setup Node.js | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: '20' | ||
| node-version: '22' | ||
| cache: 'pnpm' |
Member
Author
There was a problem hiding this comment.
actions/setup-node resolves a bare major like '22' to the latest available 22.x release, never an early patch — so it can't land on a pre-22.5 runner that lacks node:sqlite. Pinning to '22.5.0' would actually be counterproductive: it forces CI onto the oldest supported patch, where node:sqlite is at its most experimental. Keeping '22' (newest 22.x, always ≥ 22.5) is the safer choice, so leaving this as-is.
Schema introspection reads small integer flags (notnull, pk, unique) that must be plain numbers for `=== 1` / `> 0` comparisons to work, while executeSQL needs BigInt to preserve 64-bit user-data precision. The migration left this split implicit and inconsistent: some methods used this.prepare() (BigInt) and others this.db.prepare() (number), and getTableSchema was on the wrong side — its `notnull === 1` check silently failed against BigInt. - Add private queryAll<T>/queryOne<T> helpers for introspection (plain numbers, centralizing the repeated `as unknown as X` casts and the not-connected guard) - Route all introspection (getTables, getViews, tableExists, getTableIndexes, getTableSchema) through them; getTableSchema now compares plain numbers - Keep prepare() (BigInt) for executeSQL only, with a clarified doc comment No behavior change for covered cases; corrects the latent NOT NULL detection for non-PK columns in getTableSchema. All SQLite tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The engine floor now applies to running DBHub via npm/npx (node:sqlite), not just SQLite. Note it in the README quick-start and dev sections, and add a callout in installation.mdx clarifying Docker users don't need local Node. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The experimental-warning suppression was a top-level side-effect import, so loadConnectors patched process.emitWarning globally at every startup — even for instances that never use SQLite. Now that node:sqlite is loaded lazily inside connect(), the hook can be too: convert the suppress module to an idempotent function and call it right before the dynamic import. Non-SQLite processes no longer touch process.emitWarning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #317.
What
Replaces
better-sqlite3with Node's built-innode:sqlitemodule for the SQLite connector. This eliminates native C++ compilation (node-gyp) and prebuild-binary friction — the pain points raised in #317 (Alpine/musl Docker, newer Node majors, locked-down environments). SQLite now needs no driver package and is always available.API mapping
new Database(path, {readonly})new DatabaseSync(path, {readOnly})db.defaultSafeIntegers(true)(connection-level)stmt.setReadBigInts(true)per statement, centralized in a privateprepare()helperdb.inTransactiondb.isTransactionBehavior preserved
run()returnschangesas BigInt oncesetReadBigIntsis on; better-sqlite3 returned a plain number, so the row-count arithmetic is normalized viaNumber().enableForeignKeyConstraints: false) — node:sqlite defaults them on, better-sqlite3 (and SQLite itself) does not.Build/runtime gotchas fixed
node:prefix. tsup 8.5 defaultsremoveNodeProtocol: true, rewritingnode:sqlite→sqlite(unresolvable builtin) so the connector silently failed to load in the built binary. Fixed withremoveNodeProtocol: false.ExperimentalWarning. node:sqlite emits it at module-load time, and a static import is hoisted above suppression code after bundling. Fixed by a targeted warning filter (suppress-experimental-warning.ts) imported first, plus loadingnode:sqlitevia a non-hoisted lazyawait import()insideconnect().Other changes
better-sqlite3+@types/better-sqlite3; remove its native-build approvals frompnpm-workspace.yaml.engines.nodeto>=22.5.0; CIrun-testsNode 20 → 22.Requires Node.js >= 22.5.0 (node:sqlite availability). Node 20 is EOL as of April 2026, so the floor bump is reasonable, but it should be flagged in release notes / a suitable version bump.
Verification
--demomode, returns correct data, no ExperimentalWarning?parameter binding verified at runtimetscclean for all touched files🤖 Generated with Claude Code