feat(typescript): add queryBuilder pattern for query parameter serialization#15026
feat(typescript): add queryBuilder pattern for query parameter serialization#15026Swimburger wants to merge 30 commits intomainfrom
Conversation
…alizer Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
🌱 Seed Test SelectorSelect languages to run seed tests for:
How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR. |
…rayFormat through Fetcher.Args Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…pport Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…lse params Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…p instead of single value Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…n type in query param array formats Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ter serialization Replace per-parameter config map (queryParameterArrayFormats) with a fluent QueryStringBuilder that matches the C# architecture. The generator now emits chained .add() / .addComma() calls so serialization format is decided at code-gen time rather than runtime. - Add QueryStringBuilder class with .add(), .addComma(), .mergeAdditional(), .build() - Update Fetcher.Args to accept pre-built queryString instead of queryParameterArrayFormats - Update all endpoint implementations to use builder pattern via getQueryStringExpression() - Remove getQueryParameterArrayFormats helper functions from endpoint implementations - Regenerate all ts-sdk seed output Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ral comma separators The addComma() method correctly produces literal comma separators (tags=a,b,c) for OpenAPI explode:false parameters. The test expectations were incorrectly expecting %2C-encoded separators. Fixed all 4 affected test assertions. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…new QueryStringBuilder() Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…minate arrayFormats tracking Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…e: "comma" } API Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…Url (now handled by queryBuilder) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… now matches qs library Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ests - Modify V1 auto-example generator to wrap scalar examples in list containers when query parameter has allowMultiple=true, so wire tests exercise the array serialization path (comma format) - Modify convertExampleEndpointCall to handle array examples for allow-multiple query params from Fern definition examples - Add example values to query-parameters-openapi test fixture - Regenerate all affected ts-sdk seed fixtures Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Docs Generation Benchmark ResultsComparing PR branch against latest nightly baseline on
Docs generation runs |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ples Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ll snapshots Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| description: "should handle undefined values in comma format", | ||
| input: { items: ["a", undefined, "c"] }, | ||
| options: { arrayFormat: "comma" }, | ||
| expected: "items=a,,c", |
There was a problem hiding this comment.
does this match qs behavior?
There was a problem hiding this comment.
Already fixed in commit edfea2cdc7e — the tests now expect skip behavior matching qs:
{
description: "should skip null values in comma format",
input: { items: ["a", null, "c"] },
options: { arrayFormat: "comma" },
expected: "items=a,c", // was "items=a,,c"
},
{
description: "should skip undefined values in comma format",
input: { items: ["a", undefined, "c"] },
options: { arrayFormat: "comma" },
expected: "items=a,c", // was "items=a,,c"
},
{
description: "should produce empty string for all-null array in comma format",
input: { items: [null, undefined] },
options: { arrayFormat: "comma" },
expected: "",
},The qs library with { arrayFormat: "comma" } skips undefined values entirely, so ["a", undefined, "c"] → "a,c". Our implementation now matches this.
| description: "should handle null values in comma format", | ||
| input: { items: ["a", null, "c"] }, | ||
| options: { arrayFormat: "comma" }, | ||
| expected: "items=a,,c", |
There was a problem hiding this comment.
does this match qs behavior?
There was a problem hiding this comment.
Same as above — already fixed in edfea2cdc7e. The diff shown here is from an older commit; the latest code skips null/undefined entirely (matching qs library behavior).
| .add("limit", _queryParams.limit) | ||
| .add("id", _queryParams.id) | ||
| .add("date", _queryParams.date) | ||
| .add("deadline", _queryParams.deadline) | ||
| .add("bytes", _queryParams.bytes) | ||
| .add("user", _queryParams.user) | ||
| .add("userList", _queryParams.userList) | ||
| .add("optionalDeadline", _queryParams.optionalDeadline) | ||
| .add("keyValue", _queryParams.keyValue) | ||
| .add("optionalString", _queryParams.optionalString) | ||
| .add("nestedUser", _queryParams.nestedUser) | ||
| .add("optionalUser", _queryParams.optionalUser) | ||
| .add("excludeUser", _queryParams.excludeUser) | ||
| .add("filter", _queryParams.filter) |
There was a problem hiding this comment.
All the non-comma style queryparams could be added in one method, like .addMany or something similar
There was a problem hiding this comment.
Implemented in b265758 — added addMany(params: Record<string, unknown>) to QueryStringBuilder. The generator now emits:
core.url.queryBuilder()
.addMany(_queryParams) // all params with default repeat format
.add("tags", _queryParams.tags, { style: "comma" }) // comma overrides
.add("optionalTags", _queryParams.optionalTags, { style: "comma" })
.mergeAdditional(requestOptions?.queryParams)
.build()This replaces 15+ individual .add() calls with a single .addMany() + only the comma-style overrides. All 207 seed fixtures regenerated.
…tion Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…s to all ts-sdk seed fixtures Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- convertParameters.ts: extract array examples via getParameterExample() - ExampleEndpointFactory.ts: accept non-primitive query param examples (arrays, objects) instead of aborting the entire endpoint - buildEndpointExample.ts: preserve full array examples instead of flattening to first element - Wire test now uses tags: ["ACCESS_GRANTED", "COPY"] from spec Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…i examples Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ally Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Description
Refactors TypeScript query parameter serialization to use a fluent
queryBuilder()pattern, matching the C# generator architecture. The generator now emits a chained builder (core.url.queryBuilder().addMany(…).add(…, { style: "comma" }).build()) so each parameter's serialization format (repeat vs comma) is decided at code-gen time rather than runtime via a config map.OpenAPI query parameters with
explode: falseandstyle: formuse.add("tags", tags, { style: "comma" })to produce comma-separated values (e.g.,tags=a,b,c), while all other array parameters use the default repeat format (tags=a&tags=b&tags=c).Each
.add()call eagerly serializes the parameter immediately (matching the C#QueryStringBuilderpattern), storing the result in aMap<string, string>— no separate format tracking map is needed. Non-comma params are added in bulk via.addMany(), then comma-style params override their keys individually.Changes Made
Runtime (
core-utilities)src/core/url/QueryStringBuilder.ts— Exports aqueryBuilder()factory function (class is private). Provides three fluent methods:.add(key, value, options?)— eagerly serializes a single parameter viatoQueryString(), with optional{ style: "comma" }for comma-separated arrays.addMany(params)— batch-adds all entries from aRecord<string, unknown>using the default repeat format; null/undefined values are silently skipped.mergeAdditional(params?)— overrides existing keys with runtimerequestOptions.queryParams(last-write-wins viaMap.set).build()— joins pre-serialized parts with&src/core/url/index.ts— Re-exportsqueryBuilder.src/core/url/qs.ts— Added"comma"to theArrayFormattype. Comma format encodes each value individually before joining with literal commas (so commas within values become%2Cwhile separator commas remain literal). Null/undefined values within arrays are skipped entirely (matchingqsnpm library behavior). The API matches theqsnpm library — no custom extensions.src/core/fetcher/Fetcher.ts— AddedqueryString?: stringtoFetcher.Args. When provided, the fetcher uses the pre-built string instead of callingcreateRequestUrlfor query params.src/core/fetcher/createRequestUrl.ts— Removed the oldarrayFormatsthird parameter; the function now only takesbaseUrlandqueryParameters.Generator (
commons+client-class-generator)commons/src/core-utilities/UrlUtils.ts— ExposesqueryBuilder._invoke()so generated code can call the factory function.commons/src/core-utilities/Fetcher.ts— AddedqueryStringproperty to generatedFetcher.Argsinterface and_invokeemission.endpoints/utils/GeneratedQueryParams.ts— NewgetQueryStringExpression(context)method emits the builder chain. Separates params into non-comma (batched via.addMany(_queryParams)) and comma (individual.add(key, value, { style: "comma" })overrides). Usesexplode === falseto select comma style.endpoint-request/Generated{Default,Bytes,FileUpload}EndpointRequest.ts— UpdatedgetFetcherRequestArgs()return types and implementations to includequeryStringfrom the builder.endpoints/default/GeneratedDefaultEndpointImplementation.ts—buildFetcherArgs()now passesqueryStringwhen present.Wire test example generation (IR pipeline)
packages/cli/generation/ir-generator/src/converters/services/convertExampleEndpointCall.ts— When converting Fern definition examples forallow-multiplequery params with array values, each element is now converted individually and wrapped in a list container (ExampleContainer.list).test-definitions/fern/apis/query-parameters-openapi/openapi.yml— Addedexamplevalues totags(["ACCESS_GRANTED", "COPY"]) andoptionalTags(["DELETE", "MOVE"]) parameters so wire tests exercise the comma format with real array data.Snapshot updates
packages/cli/generation/ir-generator-tests/.v57-to-v56andv58-to-v57migration snapshots.dynamic.test.ts.snap,ir.test.ts.snap,validate.test.ts.snap,diff.test.ts.snap, anddynamic.json.Seed output
ts-sdkseed fixtures. Generated code now uses theaddMany+ comma override pattern:Changelog
featentry undergenerators/typescript/sdk/changes/unreleased/.fixentry underpackages/cli/cli/changes/unreleased/for array example support.Human Review Checklist
addMany+ comma override pattern: Comma-style params are added twice — first via.addMany(_queryParams)(repeat format) then overridden via.add(key, value, { style: "comma" }). This relies onMap.setlast-write-wins. Verify this is acceptable vs. filtering comma params out of theaddManycall.explode: false. Confirm this is acceptable vs. only emitting the builder when at least one comma param exists.queryParameterscomputation: Generated code still computes thequeryParametersobject and thequeryStringbuilder chain. WhenqueryStringis present, it takes precedence in the fetcher, making the object value unused at runtime. Verify this dual-emit is acceptable or if the legacyqueryParameterspath should be removed.mergeAdditionaloverride semantics: WhenrequestOptions.queryParamscontains a key that was already.add()-ed,mergeAdditionalusesMap.set(last-write-wins). Confirm this is the desired precedence.a%2Cb,cfor["a,b", "c"]. The realqslibrary encodes commas too (a%2Cb%2Cc). This divergence is intentional (unambiguous output), but worth confirming.["a", null, "c"]→a,c), matchingqslibrary behavior. Verify this is the desired behavior.Testing
QueryStringBuilder(34 tests:.add()repeat + comma styles,.addMany()batch, chaining,.mergeAdditional(), edge cases, end-to-end scenario)qs.ts(12 tests)qs.tsandcreateRequestUrl.tstests passing{ style: "comma" }forexplode: falseparamsts-sdkseed fixtures regenerated and passingLink to Devin session: https://app.devin.ai/sessions/5b96d911886f499cbfabba83155f56a6
Requested by: @Swimburger