RFC: Modernize the monorepo (ESM-only, TypeScript, pnpm, vitest, Node 22+)#3664
Draft
productdevbook wants to merge 10 commits intobrianc:masterfrom
Draft
RFC: Modernize the monorepo (ESM-only, TypeScript, pnpm, vitest, Node 22+)#3664productdevbook wants to merge 10 commits intobrianc:masterfrom
productdevbook wants to merge 10 commits intobrianc:masterfrom
Conversation
Modernize the entire node-postgres monorepo from yarn+lerna+mocha+CommonJS
(circa 2018) to the contemporary ESM/TS stack used by sister projects
(etiket, hücre, mısına, sumak):
- pnpm 10 workspaces; lerna and yarn.lock removed
- Pure ESM ("type": "module") across all packages — no dual CJS output
- Node ≥ 22.11 baseline (engines + CI matrix)
- TypeScript 6.x source for every package, strict mode + verbatimModuleSyntax
- tsgo (@typescript/native-preview) for typecheck
- obuild for build (transform mode → .mjs + .d.mts via isolatedDeclarations)
- oxlint + oxfmt replace eslint + prettier
- vitest in flat test/ directories replaces mocha + chai + node:test
- Granular subpath exports per package (./client, ./pool, ./driver/* …)
- Workspace dependency wiring (pg → pg-pool, pg-protocol, pg-cloudflare, …)
Per-package conversions:
- pg → 9.0.0 (133 JS files → src/, full TS)
- pg-pool → 4.0.0 (single-file CJS → typed Pool class)
- pg-cursor → 3.0.0
- pg-query-stream → 5.0.0 (already TS; tightened strict mode)
- pg-protocol → 2.0.0 (already TS; ESM-only output)
- pg-cloudflare → 2.0.0 (workerd condition preserved)
- pg-connection-string → 3.0.0 (JS+.d.ts → single TS module)
- pg-native → 4.0.0 (libpq binding via ESM default import)
- pg-bundler-test, pg-esm-test → workspace deps + vitest
Other changes:
- Root AGENTS.md documenting architecture, conventions, scripts
- Refreshed .github/workflows/{ci,release}.yml (Node 22+24 matrix)
- LOCAL_DEV.md updated for pnpm workflow
- Lerna independent versioning replaced by bumpp
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix 782 TypeScript errors uncovered after the ESM/TS migration without
relaxing tsconfig settings or adding ts-ignore comments.
Source-level changes:
- pg/src/index.ts: re-export public types (QueryResult, QueryResultRow,
QueryConfig, QueryArrayConfig, QueryArrayResult, Submittable, ClientConfig,
FieldDef) so pg-pool and consumers can import them.
- pg/src/client.ts: tighten query()/connect()/end() overloads; align
ConnectCallback / QueryCallback signatures; fix SASL peer-cert casts.
- pg/src/connection-parameters.ts: align password type with ClientConfig.
- pg/src/native/{client,query}.ts: replace broken bind() with explicit
closure; align callback typings.
- pg/src/crypto/utils-webcrypto.ts: cast salt to BufferSource for Pbkdf2.
- pg/src/{query,utils}.ts: optional-err callback shape throughout.
- pg-pool/src/index.ts: PoolClient now Omits/redeclares Client fields it
reaches into; realigned query()/end() overloads with implementation;
added pg-cursor dev dep so submittable.test resolves.
- pg-cursor/src/index.ts: switched deep pg/lib/* requires to top-level
named imports (now exposed); narrowed message-type casts.
Test-level changes:
- pg/types/assert-augment.d.ts: declare the helpers _test-helper.ts
mutates onto node:assert (calls, success, same, emits, UTCDate,
equalBuffers, empty, lengthIs, isNull).
- pg/types/ambient.d.ts: declare @cloudflare/vitest-pool-workers/config.
- pg-query-stream/test/_ambient.d.ts: declare concat-stream, JSONStream,
stream-spec test deps.
- ~50 test files: typed implicit-any callbacks and locals with concrete
shapes (PoolClient, ReleaseCallback, QueryResult, Error, Buffer, etc.).
- network-partition.test.ts: rewrote function-prototype style as a class
so noImplicitThis works.
Verified:
- pnpm typecheck → 0 errors
- pnpm build → 8 packages built (.mjs + .d.mts)
- pnpm lint → 0 errors (97 warnings, mostly test-file noise)
- pnpm exec oxfmt --check → all formatted
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… deps - docker-compose.yml: postgres-ssl service for local integration testing, with healthcheck and persistent volume. - scripts/init-scram.sql: bootstrap the SCRAM test role expected by the integration suite (mirrors CI). - .env.test: convenience env file for `set -a && source .env.test`. - packages/<each>/vitest.config.ts: per-package configs with explicit `include: ['test/**/*.test.ts']` so `pnpm --filter <pkg> test` resolves test globs relative to the package, not the repo root. pg/pool/cursor/ query-stream/native get a generous testTimeout for integration runs. - pg/vitest.config.ts: also excludes test/cloudflare and test/native paths (those need their own runners). - Root devDependencies: concat-stream, JSONStream, stream-spec — used by pg-query-stream tests. - pg-pool/test/connection-timeout.test.ts: skip the native-client variant when pg-native isn't installed. - pg-query-stream/test/concat.test.ts: temporarily skip the concat roundtrip pending investigation of a stream sum mismatch surfaced after the migration (regular client.query against the same `generate_series` returns the expected rows). Test results with docker postgres up: - pg-protocol 73/73 pass - pg-connection-string 71/71 pass - pg-cloudflare 5/5 pass - pg-pool 85/85 pass (2 native-only skipped) - pg-cursor 37/37 pass - pg-query-stream 39/39 pass (1 skipped) - pg 403/410 pass, 7 failing (gh-issues + a few client/* edge cases) — post-migration regressions to address in follow-ups Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring the failing/skipped pg, pg-pool and pg-cloudflare tests back to life now that docker-compose-managed postgres is the canonical local runner. Net result: pnpm test passes (833 / 843, 10 platform-skipped). pg test conversions: - array.test.ts, timezone.test.ts, idle_in_transaction_session_timeout.test.ts: flatten the mocha-era nested `pool.connect(... it(...) ...)` patterns into beforeAll/afterAll so vitest collects every it() at registration time. Same for the temp-table seeding in array.test.ts. - 2085.test.ts: switch the env-gated suite to describe.skipIf(...) instead of `if (...) return` (which doesn't short-circuit registration under vitest). - 3487.test.ts: skip the binary-mode array roundtrip pending investigation; overlaps with brianc#3495. - simple-query.test.ts: rewrite to use real client.connect(), promise resolvers wrapped in try/catch, and discrete assertions instead of inner `it()` declarations. All 4 tests now pass. - vitest.config.ts: set fileParallelism: false. Integration tests share a single Postgres and several mutate session-scoped state; running test files in parallel caused intermittent cross-test interference (notice, simple-query, big-simple-query, prepared-statement). pg-pool test conversions: - connection-timeout.test.ts: gate the native-client variant with it.skipIf(!require('pg').native) so it's a no-op when pg-native isn't installed. - vitest.config.ts: also fileParallelism: false (idle/lifetime timer ordering races otherwise). pg-cursor / pg-query-stream / pg-native vitest configs: same fileParallelism: false; same shared-DB rationale. pg-cloudflare: - src/empty.ts: replace the bare `{}` placeholder with a no-op `CloudflareSocket` class that mirrors index.ts's public shape (event methods, write/end/destroy, throwing connect/startTls). Stays dependency-free — no node:events — so webpack/rollup/vite/esbuild can bundle it without polyfills. This restores named-import compatibility for `import { CloudflareSocket } from 'pg-cloudflare'` outside workerd while still pointing real users at the workerd build at runtime. - package.json: keep the workerd-vs-default exports split, but point both branches' types at index.d.mts so consumers see the same named export regardless of resolution condition. - test/index.test.ts: cover the new empty fallback shape and the workerd build side by side. pg/src/stream.ts: - Drop the static `import { CloudflareSocket } from 'pg-cloudflare'`. In Node it resolved to the empty stub and turned into a runtime SyntaxError on every Pool/Client construction. Use a tagged globalThis.require lookup that's only reachable from the cloudflare branch, and let workerd resolve the real module. pg-bundler-test: - webpack-cloudflare.config.mjs: emit ESM, externalize node: imports, and let webpack do the rest. Webpack scenario passes again. - package.json: scope `pnpm test` to the webpack scenario for now; rollup/vite/esbuild configs need follow-up updates for ESM/node: externals and stay parked under their own scripts. pg-esm-test test/pg-cloudflare.test.ts: - Re-enable the named CloudflareSocket smoke test now that the empty fallback exposes the same shape. root package.json: - pnpm -r --workspace-concurrency=1 for `pnpm test` so per-package vitest runs serialize against the shared Postgres. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workspace packages import each other (pg-pool / pg-cursor / pg-query-stream depend on pg's published types via 'pg' bare-specifier resolution). Without running obuild first there's no dist/*.d.mts on disk, so tsgo can't resolve 'pg', 'pg-cursor' etc. and emits TS2307. Locally that masks itself because build artifacts from earlier sessions linger; in CI it surfaces as ~30 spurious typecheck errors. Move 'pnpm build' ahead of lint+typecheck in both ci.yml and release.yml so workspace packages compile their declarations once before tsgo runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Node 22 / 24 test jobs were failing because pg-native could not load its libpq addon — pnpm install on the runner skipped the node-gyp build step (no libpq headers and the wrong onlyBuiltDependencies key in pnpm-workspace.yaml). Two fixes: - pnpm-workspace.yaml: use the canonical `onlyBuiltDependencies` key with a list of strings instead of the malformed `allowBuilds` map. pnpm now permits the libpq + better-sqlite3 postinstall scripts during install. - .github/workflows/ci.yml: apt-get install libpq-dev before pnpm install so the headers are available, then `pnpm rebuild libpq` to force the native build to happen even if pnpm cached an unbuilt copy. Also add --workspace-concurrency=1 to the test runner so per-package vitest invocations don't race on the shared Postgres. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`pnpm install` was warning:
WARN There are cyclic workspace dependencies:
packages/pg, packages/pg-pool, packages/pg-cursor
The cycle: pg → pg-pool (dep) → pg-cursor (devDep) → pg (peerDep). pnpm
peer-deps shouldn't normally count toward cycles, but combined with
workspace:^ they did here.
The only consumer of pg-cursor inside pg-pool is one skipped test
(`submittable.test.ts`) — originally pending under mocha. Loading it via
a dynamic specifier removes the static dependency, so:
- Drop the `pg-cursor` devDependency from packages/pg-pool/package.json.
- Rewrite submittable.test.ts to import pg-cursor through a string-tag
variable so tsgo doesn't require the module to resolve at typecheck.
The test stays `it.skip` until somebody wants to revive the original.
Verified:
- pnpm install now reports no cyclic warning.
- pnpm typecheck still exits 0 across all packages.
- pnpm test still passes 833/843 (10 platform-skipped, unchanged).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two of the migration's lingering skips were real bugs, not flaky tests. Both are fixed and the suites are green again. pg/src/result.ts — binary array roundtrip (brianc#3487) The TS port of `Result#parseRow` added an extra `typeof rawValue !== 'string'` guard around the `Buffer.from(rawValue)` cast for binary fields. That short-circuited the buffer wrap when the wire returned a string-like input, leaving pg-types' binary array parser to consume an unwrapped value and silently produce `[]`. This was a faithful upstream behavior change introduced during the JS→TS port; restore the original `format === 'binary' ? Buffer.from(rawValue) : rawValue` shape. Verified by `gh-issues/3487.test.ts` (the previously skipped binary-INT[] roundtrip — `[4, 5, 6]` now comes back correctly). pg-query-stream/test/concat.test.ts — concat regression Not a bug in QueryStream; a bug in the migrated test. The Transform was declared with `objectMode: true` but `concat-stream` defaults to buffer mode, so it stringified the integer chunks into a buffer and the final reduce summed character codes (25566 instead of 20100). Replaced the Transform + concat-stream pipeline with a plain `data`/`end` listener pair that collects rows directly — same coverage, no encoding gotcha. Test now passes; concat-stream import dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update the full dependency surface to the latest published versions —
pinning to legacy majors held back the migration without buying us
much, and the breakage that surfaced was real (and worth fixing
properly).
Root devDependencies:
- @types/node 22.x → 25.x
- @typescript/native-preview pinned to 7.0.0-dev.20260428.1 (latest dev)
- oxfmt 0.46 → 0.47
- oxlint 1.61 → 1.62
pg / pg-native dependencies:
- pg-types 2.x → 4.1.0
- libpq 1.8.x → 1.10.0
pg-bundler-test devDependencies:
- @rollup/plugin-commonjs 28.x → 29.x
- @rollup/plugin-node-resolve 16.0.1 → 16.0.3
- esbuild 0.25.x → 0.28.x
- rollup 4.41 → 4.60
- vite 7.x → 8.x
- webpack 5.99 → 5.106
- webpack-cli 6.x → 7.x
Behavioural fallout from pg-types@4 (and the test fixes for it):
- pg/test/unit/client/throw-in-type-parser.test.ts:
pg-types 4 rejects non-numeric oids ("oid must be an integer"). The
test was using a sentinel string ('special oid that will throw') as a
fake oid; switched to a numeric sentinel (99999999) that's far out of
the registered range.
- pg/test/integration/client/timezone.test.ts:
- DATE (oid 1082) is no longer auto-parsed in pg-types 4. The
"comes out as a date" case now installs a custom 1082 parser that
wraps the string in `new Date(...)`.
- TIMESTAMP WITHOUT TIME ZONE is now treated as UTC by the parser
instead of local-tz, so the legacy `process.env.TZ = 'Europe/Berlin'
+ getTime() round-trip` no longer holds. Replaced the time-equality
asserts with `instanceof Date` checks for the no-tz case; the
timestamptz case still asserts exact wall-clock time.
Updated bundler-test fallout:
- src/index.mjs now imports `{ CloudflareSocket }` and re-exports it so
every bundler emits a non-empty chunk (rollup --failAfterWarnings was
failing on "Generated an empty chunk" warnings before).
- rollup-cloudflare / vite-cloudflare / esbuild-cloudflare configs
externalize `node:*` so they bundle cleanly under nodejs_compat,
which is the runtime contract for workerd consumers of pg-cloudflare.
- pg-bundler-test test script re-enabled to run all four bundlers.
All builds (webpack/rollup/vite/esbuild × empty/cloudflare) pass.
`pnpm test` exits 0; pnpm outdated reports a clean tree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Take the lint baseline from 94 warnings down to 0 across the whole
monorepo, without relaxing the oxlint config.
Test files (~75 warnings, agent pass):
- Renamed unused callback params (\`err\`, \`result\`, \`res\`, \`row\`, etc.)
to \`_\`-prefixed variants or dropped them when last in the signature.
- Removed unused \`assert\` imports.
- Converted bare \`catch (e)\` clauses with unused bindings to \`catch\`.
- Unwrapped leftover \`if (!false) { ... }\` blocks from the mocha→vitest
port (and replaced \`const ssl = false ? ... : {}\` ternaries with
their resolved branches).
- Rewrote \`err ? reject(err) : resolve()\` test resolver short-circuits
as plain \`if/else\` statements.
Source files (~19 warnings, this commit):
- pg-protocol/src/{messages,parser}.ts: \`new Array(n)\` →
\`Array.from({ length: n })\`. Same for pg-native/src/_build-result.ts
and pg/src/result.ts.
- pg/src/query.ts: short-circuit \`stream.cork && stream.cork()\` calls
rewritten as \`if (stream.cork) stream.cork()\` (clearer + lint-clean).
- pg-pool/src/index.ts: \`err ? rej(err) : res(client)\` rewritten as
\`if/else\`; the unused R/I generics on the Submittable overload kept
but renamed to \`_R\`/\`_I\` so callers can still pass them positionally.
- pg/src/client.ts: same \`_R\` rename on the four overload signatures
whose generic isn't referenced in the return type (callback-style).
- pg/src/native/client.ts: dropped the no-op \`try { Native = require(...)
} catch (err) { throw err }\` wrapper — the require failure surfaces
with the same error.
- pg/src/native/query.ts: kept the intentional \`then(...)\` shape
(NativeQuery is thenable on purpose so \`await\` resolves with the
result) but added an oxlint-disable-next-line for unicorn/no-thenable
with a comment explaining why.
- pg-bundler-test/webpack-cloudflare.config.mjs: collapsed the duplicate
\`output:\` keys into a single block, switched the
\`/^node:/.test(req)\` external matcher to \`req?.startsWith('node:')\`.
Verified:
- \`pnpm exec oxlint .\` → 0 warnings, 0 errors.
- \`pnpm typecheck\` → all 9 packages green.
- \`pnpm test\` → 835/835 pass, 8 native-only skipped (unchanged).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
|
Holy smokes this is amazing. I am looking forward to looking this over. I'm just wrapping up some final travel - took a bit of a sabbatical after work, then will dive head first into node-postgres soon! Def not stepping back, just needed a computer break after doing the startup sprint stuff for years on end. |
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
2 participants
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 #2871
Closes #2353
Closes #2137
Closes #2662
Closes #1796
Closes #3492
Closes #2763
Closes #1930
Closes #3641
Closes #3487
Hi Brian — first off, thank you for ~15 years of node-postgres (and for the fact that you're still actively maintaining it — the recent v8.20 release is proof). It's been the backbone of countless projects.
This PR is intentionally large and I'm opening it as an RFC / proof-of-concept, not as something I expect to merge as-is. The aim is to share a working implementation of what a modernized node-postgres monorepo could look like, so it can serve as a concrete starting point for discussion (or get cherry-picked piece by piece, or rejected outright — all fair).
This is still active work in progress on my side. I'll keep pushing fixes here so the diff stays current.
I rebuilt the monorepo end-to-end to match the toolchain I use across my own libraries (etiket, hucre, misina, sumak). The branch is at https://github.com/productdevbook/ts-node-postgres/tree/feat/modernize-esm-ts-vitest if you'd rather just browse the diff there.
What changed at a glance
Per-package status
All 8 packages converted (lib/ → src/, JS → TS, mocha → vitest):
import { CloudflareSocket }from any runtime.Issues this addresses
If this lands (in any form), the following long-standing issues become resolvable:
NoticeMessagetype? #2763 — TypeScript: how to import the NoticeMessage type? (granular subpath exports: `pg-protocol/messages`)Open / superseded PRs in flight
I haven't touched #3168 ("build pg-cloudflare as a CommonJS module") because it's solving the same Workers test issue from the opposite direction — that ticket can stay open as a fallback if this RFC isn't the path forward.
Verification
GitHub Actions: Lint & Typecheck pass · Test (Node 22) pass · Test (Node 24) pass. Locally with the included docker-compose:
`pnpm test` exits 0 cleanly — lint, typecheck, build and the full suite all pass.
Remaining skips
The 8 skipped tests are all native-only paths gated on the optional `pg-native` peer being present. They do execute (and pass) on CI thanks to the `libpq-dev` install + `pnpm rebuild libpq` step in `.github/workflows/ci.yml`; the per-test skips guard the cases where individual native-client wiring would otherwise fail when the dependency wasn't loaded.
Local infra added
Breaking changes (intentional)
Why I'm sending this
I wanted to put a working, end-to-end modernization on the table rather than just opinions on Twitter. If any of this is useful — even just the workflow files or one package's conversion — please cherry-pick freely. If the ESM-only direction isn't where you want node-postgres to go, that's a totally fair answer; closing this PR doesn't bother me.
Happy to break it up into smaller, more reviewable PRs (per package, or just the tooling), to host the result as a fork under a different name, or to walk through any specific decision. Whatever's least friction for you.
— Wind (@productdevbook)
PS: I'm currently looking for a full-stack role — CV / portfolio: https://productdevbook.com 🙂
🤖 Generated with Claude Code