Repository navigation
feat(postgres): run queries on slonik - #121
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Important
The interceptor refusal fires after Slonik has already run the statement, so a write can commit and still be reported to the app as an error. Details inline.
Reviewed changes
I reviewed the full diff. I also checked the parts of Slonik 49.12.0 the PR depends on against its published tarballs (slonik, @slonik/sql-tag, @slonik/pg-driver):
- Slonik through its own tag:
.run()'s Postgres branch now sends a Slonik pool, connection, transaction or savepoint toslonik()insrc/decode.ts. That function builds acreateSqlTag({ typeParsers })tag whose parsers return each value unchanged, splits the$ntext into the tag's template, and decodes the rows by column kind. - Loading
slonik: the app'sslonikis loaded withcreateRequirefromargv[1], then from the working directory. Slonik 49 is ESM-only on Node 24 or later, so this relies onrequire(esm). Its tokens useSymbol.for, so a tag from a second copy of Slonik is still accepted. - Parameters:
slonikParamturns each value into one the tag accepts:undefinedbecomesNULL, bytes become\xhex, arrays become literals, and aDatebecomes its ISO string. A hand-written query that uses a parameter twice is refused. - Type parsers: Slonik's per-query parsers also cover each type's array type (
typarrayin@slonik/pg-driver), sonumeric[]andtimestamptz[]arrive as text, as the PR relies on. try(): reads pg's error from thecauseon Slonik's error wrapper.- Lab and docs: a driver-matrix job and type-check cases for
slonik@49.12.0, plus docs, README, and an optional peerslonik >=49.12.0.
ℹ️ Apps that bundle slonik cannot run Reylo's queries on it
loadSlonik can only find slonik as a package on disk, starting from argv[1] or the working directory. An app that bundles slonik into its server output and ships no node_modules (an esbuild-bundled Lambda, for example) fails every Slonik query with the "could not load" TypeError, even though the pool works. The new docs section on the Postgres page doesn't mention this.
Technical details
# A bundled slonik is not resolvable by loadSlonik
## Affected sites
- src/decode.ts:443-458 — `loadSlonik` resolves "slonik" only from disk (`createRequire(argv[1] | cwd)`)
- site/content/docs/postgres.mdx:924 — the slonik section lists requirements but not this one
## Required outcome
- A user whose deployment bundles slonik learns before deploying that slonik must be left external, so it can be resolved at run time.
## Open questions for the human
- Is a docs note enough, or should the refusal message suggest marking slonik external in the bundler?claude-opus-5-5 ｜ 𝕏
|
On bundling |
There was a problem hiding this comment.
ℹ️ The prior finding is fixed. One small gap is left, noted inline.
Reviewed changes
I reviewed 0ad50b8 against the prior review at 6678654, and checked it against executeQuery.js in Slonik 49.12.0.
- Up-front interceptor refusal:
slonik()now refuses a client whose interceptors include atransformRowbeforequery()is called. It reads the interceptors fromconfiguration(a pool) orclientConfiguration(a connection, transaction or savepoint). The check on the rows' keys after the query is gone. - Tests: the refusal test puts the interceptor in both places and asserts that nothing was sent. It also checks that an interceptor with only
afterQueryExecutionstill runs, which is right: Slonik ignores whatafterQueryExecutionandbeforeQueryResultreturn. - Bundled servers: the docs and the "could not load" error now say that a bundled server must leave
slonikexternal. This covers the earlier informational note.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. The last open finding is fixed.
Reviewed changes
I reviewed 57d576d against the prior review at 0ad50b8.
transformRowAsyncrefused too:slonik()now refuses, beforequery()runs, a client that has an interceptor with eithertransformRowortransformRowAsync. It checks bothconfigurationandclientConfiguration. In Slonik 49.12.0'sexecuteQuery.js, these are the only two hooks that replaceresult.rows.- Error and docs: the
refuseSlonikRowsmessage and the interceptor sentence inpostgres.mdxnow name both hooks. - Test: the refusal test now runs both hooks on both config locations and still checks that nothing was sent.
claude-opus-5-5 ｜ 𝕏
.run(),.with()andtry()now run on a Slonik pool, a connection frompool.connect(), a transaction and a savepoint, with the values Reylo returns on every other driver. This replaces the refusal added in #91.Slonik's
query()runs only the queries itssqltag builds, and its type parsers are set on the pool's connections: with its defaults anumericarrives as a float, a timestamp as epoch milliseconds and anintervalas seconds. Slonik 49.12.0 lets a tag made bycreateSqlTag({ typeParsers })override those parsers for its own queries (gajus/slonik#824). Reylo uses that, and changes nothing in the statement.createSqlTagfrom the app's ownslonik, throughrequirereached withprocess.getBuiltinModule("node:module"), from the app's entry script and then its working directory. Animport()drew Rspack's "Critical dependency" warning in every app, Slonik or not. The tag passes through as text every built-in type that pg (pg-types4) or Slonik's preset converts, plus every type the app's pool names a parser for.slonik-sql-tag-raw'sraw(), which rebinds every$nand matches inside literals. A query defined by hand that uses a parameter twice is refused, since the tag would bind each use on its own.undefinedbindsNULL, binary data goes as\xhex text, and arrays and JSON as on pg, since the tag takes only strings, numbers, booleans, BigInts and null. Rows are decoded by each column's kind, or for a query defined by hand by its type's oid.slonik-interceptor-field-name-transformation), and a value a custom driver converted anyway, where it ignored the query's parsers.try()reads pg's error through Slonik's wrapper (InputSyntaxError,UniqueIntegrityConstraintViolationError), which keeps it ascause.slonikis an optional peer,>=49.12.0, and a dev dependency of the root, the driver matrix and the docs site.Docs: a Slonik tab in
.run()'s Postgres examples and transactions, asloniksection on the Postgres page, andslonikin the README's list.Checked:
tests/representation.test.tsruns one row through a Slonik pool with Slonik's own parsers plus an app parser onuuid. The pool, a connection and a transaction return the same row as every other driver, and the app's own query keeps its parsers.tests/execute.test.ts: the template, positional placeholders out of order, the values, a query defined by hand, and the three refusals.slonik@49.12.0, passes 110 rows over the pool, a connection, a transaction and a savepoint, with exact values, round trips andtry(). The 8 skips are the parameter-used-twice scenarios. A full run compares withRESULTS.mdwith 0 differences.