fix: open the learning database through bun:sqlite when node:sqlite is missing - #137
Merged
Merged
Conversation
…s missing
Pi's release binaries are Bun --compile executables, where import("node:sqlite")
fails with "No such built-in module: node:sqlite", so holds.db never opened and
learning was off for everyone running a release binary.
node:sqlite stays the first choice and its path is unchanged. Only when that
import fails and the process runs on Bun does src/sqlite-adapter.ts open the
file through bun:sqlite, wrapped to the DatabaseSync subset learning uses: get()
turns Bun's null miss into undefined, exec()'s result object is dropped, and
positional ? parameters pass through (learning binds no named keys, so the
$name prefixes never meet). With neither module loading, learning still turns
off behind one warning, which now names both modules.
tests/sqlite-adapter.test.ts runs everywhere against a fake with Bun's measured
shape and, under Bun (npm run test:bun), against real bun:sqlite together with
the learning tests. tests/learning-driver.test.ts runs two children on Node: one
proves the Node path still picks node:sqlite, the other blocks node:sqlite and
proves the single both-modules warning and the NOOP handle.
Bun's `create` option creates the file when it is missing; it defaults to true, so the old comment described the opposite of what the flag does.
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
1 participant
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.
Pi's release binaries are Bun
--compileexecutables. In themimport("node:sqlite")fails withNo such built-in module: node:sqlite, soholds.dbnever opened and learning features were off for everyone running a release binary.Change
node:sqlitestays the first choice, and its path is unchanged.src/sqlite-adapter.tsopens the database throughbun:sqlite, wrapped to theDatabaseSyncsubset learning uses:get()turns Bun'snullmiss intoundefined,exec()'s result object is dropped, and positional?parameters pass through unchanged.bun:sqlitekeeps the build free of new dependencies.Verification
npm run check: 1197 pass / 0 fail onmain, 1207 pass / 0 fail on this branch; typecheck and build clean.npm run test:bun(Bun 1.4.2): 40 pass / 0 fail, including the unchanged learning tests against realbun:sqlite.learning.tswithimport("node:sqlite")forced to fail opened the database throughbun:sqlite, wrote and read a hold, created the parent directory, and ran in WAL mode.node:sqliteblocked emits one warning naming both modules and falls back to the no-op handle.