fix(drizzle): treat undefined query properties as absent - #1648
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 265c4c8 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@pothos/core
@pothos/plugin-add-graphql
@pothos/plugin-complexity
@pothos/plugin-dataloader
@pothos/plugin-directives
@pothos/plugin-drizzle
@pothos/plugin-errors
@pothos/plugin-example
@pothos/plugin-federation
@pothos/plugin-grafast
@pothos/plugin-mocks
@pothos/plugin-prisma
@pothos/plugin-prisma-utils
@pothos/plugin-relay
@pothos/plugin-scope-auth
@pothos/plugin-simple-objects
@pothos/plugin-smart-subscriptions
@pothos/plugin-sub-graph
@pothos/plugin-tracing
@pothos/plugin-validation
@pothos/plugin-with-input
@pothos/plugin-zod
@pothos/tracing-newrelic
@pothos/tracing-opentelemetry
@pothos/tracing-sentry
@pothos/tracing-xray
commit: |
Query callbacks that return a conditional filter (`where: cond ? filter
: undefined`) produced a selection state carrying an explicit `undefined`
key. `deepEqual` compares key counts, so `{ where: undefined }` did not
match an equivalent selection that omitted the key, `selectionCompatible`
rejected the nested relation, and the field's selection was silently
dropped.
Normalize query objects by removing undefined-valued properties before
they are stored on selection state, compared, or passed to Drizzle. This
covers `where`, `orderBy`, `limit`, and `offset` — the mechanism is
key-agnostic, so scoping the fix to `where` alone would leave the same
bug reachable through its siblings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12c6baf to
265c4c8
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a Drizzle integration edge case where query callbacks that conditionally return undefined (e.g. where: cond ? filter : undefined) could cause nested selections to be treated as incompatible, leading to dropped field selections. It does so by normalizing query objects to remove undefined-valued properties before storing, comparing, or emitting Drizzle queries.
Changes:
- Add a shared
omitUndefinedKeysnormalization helper and apply it across selection merge/compatibility and query construction paths. - Ensure
undefinedquery properties are treated equivalently to absent properties in selection state comparisons and emitted Drizzle queries. - Add regression tests (unit + integration) to assert
undefinedquery properties never reach Drizzle runtime calls.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-drizzle/src/utils/selections.ts | Introduces omitUndefinedKeys and applies it in selectionCompatible, mergeSelection, and selectionToQuery. |
| packages/plugin-drizzle/src/utils/cursors.ts | Normalizes cursor-connection query objects so undefined properties are omitted before mapping to Drizzle. |
| packages/plugin-drizzle/src/drizzle-field-builder.ts | Normalizes relation query callback results so undefined keys don’t leak into relation with selections. |
| packages/plugin-drizzle/src/connection-helpers.ts | Normalizes the final merged connection helper query (baseQuery + selection query). |
| packages/plugin-drizzle/tests/selections.test.ts | Adds unit tests for omitUndefinedKeys behavior and compatibility/merge normalization cases. |
| packages/plugin-drizzle/tests/drizzle-field.test.ts | Adds an integration assertion that root queries omit where when it is undefined. |
| packages/plugin-drizzle/tests/drizzle-connections.test.ts | Adds integration assertions via spies that where: undefined is omitted in connection queries. |
| packages/plugin-drizzle/tests/connection-helpers.test.ts | Adds integration assertions that nested with.*.where is omitted when undefined. |
| packages/plugin-drizzle/tests/related-connection.test.ts | Adds a spy-based integration assertion that nested relation where does not reach Drizzle. |
| packages/plugin-drizzle/tests/query-path.test.ts | Adds a spy-based integration assertion that relation query callbacks don’t pass where: undefined through. |
| packages/plugin-drizzle/tests/example/schema/user.ts | Updates test schema to include where: undefined in callbacks to exercise the regression path. |
| packages/plugin-drizzle/tests/example/schema/query.ts | Updates test schema to pass where: undefined into the root query builder to validate normalization. |
| .changeset/calm-trees-filter.md | Adds a patch changeset describing the fix and impacted query keys. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Problem
A Drizzle query callback that returns a conditional filter silently breaks field selection:
The root cause is not Drizzle — Drizzle truthy-checks
whereat runtime (sqlite-core/dialect.js:495) and types it aswhere?: ... | undefined. The break is on the Pothos side:deepEqual(src/utils/deep-equal.ts:37) compares key counts, so{ where: undefined }does not match an equivalent selection that omits the key.selectionCompatiblethen rejects the nested relation and the field's selection is dropped.Fix
Normalize query objects by removing undefined-valued properties before they are stored on selection state, compared, or passed to Drizzle (
omitUndefinedKeysinsrc/utils/selections.ts), applied inmergeSelection,selectionCompatible,selectionToQuery,connection-helpers,drizzle-field-builder, andcursors.The helper is key-agnostic rather than
where-specific by design. The failing mechanism is the key-count comparison, not anything aboutwhere, soorderBy,limit, andoffsetare reachable through the identical path — verified against awhere-only version of this fix:columns/with/extrasare destructured out before the rest-spread, so they are unaffected.Tests
98 tests pass, typecheck clean. Every new assertion is a genuine regression test — reverting only
src/fails 8 integration assertions across 5 files plus 7 unit assertions inselections.test.ts. Coverage spans the merge level (it.eachoverwhere/orderBy/limit/offset), the compatibility level (selectionCompatible/stateCompatible), and end-to-end viavi.spyOnondb.query.*asserting the property never reaches Drizzle. All existing inline SQL snapshots are unchanged.🤖 Generated with Claude Code