Transform JSON / JSONB parameter keys#1169
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR extends the library’s transform system so JSON parameter serialization can apply key-casing transforms (e.g., camelCase ⇄ snake_case), and updates typings + tests accordingly.
Changes:
- Apply
transform.value.toduring JSON serialization (for JSON/JSONB parameters). - Add
value.totransform helpers for camel/pascal/kebab transforms. - Update TypeScript declarations and expand JSON transform test coverage across builds (node/deno/cjs).
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| types/index.d.ts | Updates public typings for transform value-direction helpers and serializer signatures. |
| src/types.js | Applies transform-on-JSON-serialize and wires up value.to for casing transforms. |
| src/connection.js | Passes parsed options into serializers. |
| tests/index.js | Adds coverage for JSON key transforms on parameters/results. |
| deno/types/index.d.ts | Mirrors typing changes for Deno build. |
| deno/src/types.js | Mirrors JSON serialization + transform wiring for Deno build. |
| deno/src/connection.js | Mirrors passing options into serializers for Deno build. |
| deno/tests/index.js | Mirrors JSON transform tests for Deno build. |
| cjs/src/types.js | Mirrors JSON serialization + transform wiring for CJS build. |
| cjs/src/connection.js | Mirrors passing options into serializers for CJS build. |
| cjs/tests/index.js | Mirrors JSON transform tests for CJS build. |
| cf/src/types.js | Mirrors JSON serialization + transform wiring for CF build. |
| cf/src/connection.js | Mirrors passing options into serializers for CF build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| : Object.entries(x).reduce((acc, [k, v]) => Object.assign(acc, { [fn(k)]: jsonTransform(v, column) }), {}) | ||
| : Object.getPrototypeOf(x) === Object.prototype || Object.getPrototypeOf(x) === null | ||
| ? Object.entries(x).reduce((acc, [k, v]) => Object.assign(acc, { [fn(k)]: jsonTransform(v, column) }), {}) | ||
| : x |
There was a problem hiding this comment.
only transform plain objects, to avoid transforming objects like Dates
b50c30c to
5d73037
Compare
|
@porsager I think this is ready for a review |
|
Nice @karlhorky - Only problem is that this is a breaking change, so I think the behaviour would need to be something toggled by an option?? |
|
@porsager yeah technically it's a breaking change to the current behavior. Although arguably, the original promise of the But yeah, to make the library updates less surprising, it's better to consider it as a breaking change, in case users are depending on the half-baked behavior. Maybe a solution would be to release it in a new major version |
Hey @porsager, hope you're good! 👋
Postgres.js transforms JS property names passed through
sql(...)helpers, eg. with{ transform: postgres.camel }:However, currently JSON parameter keys are not transformed 💥
Maybe this was an oversight in my previous PR for "nested transforms":
This PR adds JSON value transforms before serialization for the built-in case transforms, so JSON and JSONB parameters serialize transformed keys:
This change also applies to JSON / JSONB data created via
sql.typed()and type casts:value.totransforms for camel, pascal, and kebabAI disclosure: guided use of
gpt-5.5 highthrough Codex CLI