Skip to content

Add config option to opt-out of generated query serializer#3749

Open
cappuc wants to merge 5 commits intohey-api:mainfrom
cappuc:feat/query-serializer
Open

Add config option to opt-out of generated query serializer#3749
cappuc wants to merge 5 commits intohey-api:mainfrom
cappuc:feat/query-serializer

Conversation

@cappuc
Copy link
Copy Markdown

@cappuc cappuc commented Apr 13, 2026

Fix #3747

This PR adds a config option to disable the generated querySerializer object in sdk functions.
The generated querySerializer overrides the client querySerializer so it doen't allow to handle the serialization globally.

@bolt-new-by-stackblitz
Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Apr 13, 2026

Reviewed PR #3749. The core querySerializer opt-out feature in operation.ts and config.ts is correct — it cleanly guards the serializer generation behind plugin.config.querySerializer. Left feedback on unrelated property reordering in types.ts that inflates the diff and misplaces the new option below the deprecated section, plus a typo fix in a test description.

Task list (5/5 completed)
  • Read the diff to identify major areas of change
  • Read non-snapshot source changes in detail
  • Investigate correctness, config type, and code generation logic
  • Verify test coverage and snapshot correctness
  • Submit review

Pullfrog  | View workflow run | Triggered by Pullfrog𝕏

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 13, 2026

@cappuc is attempting to deploy a commit to the Hey API Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 13, 2026

⚠️ No Changeset found

Latest commit: 6b64b7c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. feature 🚀 Feature request. labels Apr 13, 2026
@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Apr 13, 2026

TL;DR — Adds a querySerializer boolean option to the @hey-api/sdk plugin config, letting users opt out of the auto-generated per-operation querySerializer code. Defaults to true (preserving existing behavior). Includes documentation explaining the use case and configuration.

Key changes

  • Add querySerializer config option to @hey-api/sdk — A new boolean property (default true) that, when set to false, skips emitting the schema-driven querySerializer object in generated SDK operations. Useful for users who provide their own query serialization at the client level and want leaner output.
  • Guard serializer codegen behind the new flag — The existing query-parameter serialization logic in operationStatements is now wrapped in a plugin.config.querySerializer check.
  • Document the querySerializer option — New "Query Serializer" section in the SDK plugin docs with a code-group showing both the custom client-level serializer example and the config toggle.
  • Add test snapshots for all three spec versions — New query-serializer-disabled snapshot directories for 2.0.x, 3.0.x, and 3.1.x, plus two dedicated 3.1.x test cases comparing querySerializer: true vs false output using a new parameters-serializer.json spec.

Summary | 87 files | 5 commits | base: mainfeat/query-serializer

Before: The SDK plugin always emitted per-operation querySerializer configuration derived from the OpenAPI spec; there was no way to suppress it.
After: Setting querySerializer: false in the @hey-api/sdk plugin config omits all generated querySerializer output, producing a cleaner SDK when serialization is handled externally.

When querySerializer is true (or omitted), the generated SDK call includes inline serializer config derived from the spec:

querySerializer Generated SDK call
true (default) client.get({ querySerializer: { parameters: { filter: { object: { style: 'form' } } } } }, url: '/foo', ...options })
false client.get({ url: '/foo', ...options })
Why is the types.ts diff so large? In an earlier revision, deprecated properties (asClass, classNameBuilder, etc.) were re-sorted to satisfy typescript-sort-keys. That re-ordering was reverted in a follow-up commit — the final diff now only adds the querySerializer property in its correct alphabetical position, with no other property moves.

types.ts · config.ts · operation.ts · sdk.md

Pullfrog  | View workflow run | Triggered by Pullfrog𝕏

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The core feature (guarding query serializer generation behind plugin.config.querySerializer) is correct and well-tested. However, the diff includes a large unrelated reordering of properties in types.ts that makes the PR harder to review and introduces ordering issues. Please revert the property shuffling and keep only the querySerializer additions.

Task list (5/5 completed)
  • Read the diff to identify major areas of change
  • Read non-snapshot source changes in detail
  • Investigate correctness, config type, and code generation logic
  • Verify test coverage and snapshot correctness
  • Submit review

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run𝕏

Comment thread packages/openapi-ts/src/plugins/@hey-api/sdk/types.ts Outdated
Comment thread packages/openapi-ts/src/plugins/@hey-api/sdk/types.ts
Comment thread packages/openapi-ts-tests/main/test/3.1.x.test.ts Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 18.75000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.04%. Comparing base (663e270) to head (6b64b7c).
⚠️ Report is 47 commits behind head on main.

Files with missing lines Patch % Lines
...pi-ts/src/plugins/@hey-api/sdk/shared/operation.ts 18.75% 8 Missing and 5 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3749   +/-   ##
=======================================
  Coverage   40.04%   40.04%           
=======================================
  Files         520      520           
  Lines       19243    19244    +1     
  Branches     5726     5727    +1     
=======================================
+ Hits         7705     7706    +1     
  Misses       9342     9342           
  Partials     2196     2196           
Flag Coverage Δ
unittests 40.04% <18.75%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 13, 2026

Open in StackBlitz

@hey-api/codegen-core

npm i https://pkg.pr.new/@hey-api/codegen-core@3749

@hey-api/json-schema-ref-parser

npm i https://pkg.pr.new/@hey-api/json-schema-ref-parser@3749

@hey-api/nuxt

npm i https://pkg.pr.new/@hey-api/nuxt@3749

@hey-api/openapi-ts

npm i https://pkg.pr.new/@hey-api/openapi-ts@3749

@hey-api/shared

npm i https://pkg.pr.new/@hey-api/shared@3749

@hey-api/spec-types

npm i https://pkg.pr.new/@hey-api/spec-types@3749

@hey-api/types

npm i https://pkg.pr.new/@hey-api/types@3749

@hey-api/vite-plugin

npm i https://pkg.pr.new/@hey-api/vite-plugin@3749

commit: 6b64b7c

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature 🚀 Feature request. size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

querySerializer generated by SDK overrides the client one

1 participant