Repository navigation
feat: run queries on a knex instance or transaction - #233
Conversation
A Knex instance, transaction or savepoint passes as the client of .run(): the statement runs on the connection Knex hands out, over pg, mysql2, mariadb, better-sqlite3 or sqlite3, and the values decode as on that driver. A Knex instance is a function, so it was taken for Neon's neon() and called, which built a select from a table named by the statement.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
ℹ️ No critical issues. One rough edge inline: two of the Knex drivers listed don't actually run correctly.
Reviewed changes
Reviewed the Knex support in src/execute.ts, its live tests and the docs, README and skill updates.
- Knex detection:
isKnexrecognizes a callable instance, transaction or savepoint byraw,transactionandclient.acquireConnection/releaseConnection. It runs before every other client check, so a Knex instance is no longer taken for Neon'sneon(). - Running on a Knex connection:
onKnexConnectionrefuses a statement for another database (usingclient.driverName) and refuses a transaction that has already finished. Otherwise it acquires a connection, runs the statement on the raw driver connection and releases the connection infinally. I checked this against knex 3.3.0'smakeTxClient: inside a transaction, acquire returns the transaction's own connection and release does nothing. - Types:
KnexClientis added to the Postgres, MySQL and SQLite client unions but not toSyncClientFor, so.runSync()rejects a Knex instance both at compile time and at run time. - Tests: live tests in
tests/execute.test.tscover better-sqlite3, sqlite3, pg, mysql2 and mariadb. They check pool usage, a rollback that undoes Reylo's writes, savepoints, a finished transaction and a dialect mismatch.knexis added tolab/drivers. - Docs: each driver list in
02-usage.mdxgets a Knex tab, and each dialect page gets a### Knexsection. The README and the skill mention Knex.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new bugs. The fixes are correct. One small docs gap: the new
pg.nativerefusal isn't documented yet.
Reviewed changes
This review covers the two commits since the last review. Both act on the earlier feedback about Knex drivers that Reylo cannot read.
- Knex driver allowlist:
knexDriversnow lists onlypg,mysql2,mariadb,better-sqlite3andsqlite3. Any otherdriverName, such aspgnative,mysqlorcockroachdb, is refused before a connection is acquired. A stubbed test covers this. pg.nativerefused everywhere:isPgNativeinsrc/decode.tscatches pg's nativeClientandPoolClientthrough theirnativebinding. It catches a pool of them throughClient.prototype._hasActiveQuery. I checked pg's sources: onlylib/native/client.jsdefines either of these, andlib/client.jshas neither, so pg's JavaScriptClient, Neon's and@vercel/postgres's are not affected. pg-promise withpgNative: trueis refused as well. A stubbed test covers all three.
ℹ️ The new pg.native refusal is missing from the driver docs
This refusal applies to every pg and pg-promise user, not only to Knex. An app that ran on pg.native used to get values decoded the wrong way and now gets an error. The dialect page already lists what isn't supported for each driver, such as binary: true for pg and a rewriting receive for pg-promise, but the native client isn't on that list. The rspack example also still describes using pg.native.
Technical details
# Document the pg.native refusal
## Affected sites
- site/content/docs/postgres.mdx:1217 — the `### pg` section lists `binary: true` as unsupported, but not `pg.native`
- site/content/docs/postgres.mdx:1261 — the `### pg-promise` section lists `receive`, but not `pgNative: true`
- examples/rspack/README.md:53 — says an app using `pg.native` installs `pg-native` and lists it in `externals`, but Reylo now refuses that client
## Required outcome
- The `pg` and `pg-promise` sections say that pg's native client (`pg.native`, or pg-promise's `pgNative: true`) isn't supported, in the same brief style as the existing `binary: true` line
- The rspack note no longer suggests a setup that Reylo refuses, or it says Reylo refuses that setupclaude-opus-5-5 ｜ 𝕏
|
Docs gap on pg's native client fixed in 9bbb5a9. The pg and pg-promise sections now list it as unsupported, and the rspack note no longer suggests bundling it. |
There was a problem hiding this comment.
✅ No new issues found. The docs gap from the last review is fixed.
Reviewed changes
This review covers the one docs commit since the last review. It documents the pg.native refusal.
pgsection: the Postgres page now says pg's native client,pg.native, isn't supported, next to the existingbinary: trueline, and says why: it ignores the type parsers a query passes.pg-promisesection: the page now says a database initialized withpgNative: trueisn't supported either.- Rspack example: the note no longer suggests installing
pg-nativeforpg.native. It now says Reylo refuses pg's native client.
claude-opus-5-5 ｜ 𝕏
Closes #1
A Knex instance, transaction or savepoint can now be passed to
.run(),.try()and.with(). Reylo takes one of the driver connections Knex holds, runs its compiled statement on it as it would on that driver directly, and hands the connection back. Inside a transaction it runs on the transaction's own connection. Knex's query builder is not involved, so values, nullability and cardinality behave as on the driver itself.pg,mysql2,mariadb,better-sqlite3andsqlite3..runSync()takes no Knex instance, since Knex hands out connections through a promise.Before this, a Knex instance, which is a function, was taken for Neon's
neon()and called, so Postgres answered with "relation does not exist".The tests run each driver through Knex against a live database, with
knexadded tolab/drivers. The docs gain a Knex tab in each driver list and a Knex section on each dialect page, and the README and the skill list it.