Skip to content

[typescript-fetch] fix: explode object query parameters - #24803

Merged
macjohnny merged 5 commits into
OpenAPITools:masterfrom
wiebren:fix/exploded-object-query-parameters-typescript-fetch
Sep 23, 2026
Merged

macjohnny merged 5 commits into
OpenAPITools:masterfrom
wiebren:fix/exploded-object-query-parameters-typescript-fetch

Conversation

@wiebren

@wiebren wiebren commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Bug

A query parameter whose schema is an object, with style/explode left at their defaults (form, true), must go on the wire as one parameter per entry, keyed by the property name alone. The typescript-fetch client bracketed it instead.

parameters:
  - in: query
    name: filter
    schema:
      type: object

called with {"category": "books", "createdDate:gte": "2023-01-01"}:

on the wire
expected category=books&createdDate%3Agte=2023-01-01
typescript-fetch before filter[category]=books&filter[createdDate%3Agte]=2023-01-01

Series

One of six per-language PRs for the same bug: #24797 go, #24802 python, #24803 typescript-fetch, #24867 kotlin, #24868 dart, #24869 ruby. No shared main/ code; each adds the same fixture, 3_0/exploded-object-query-param.yaml.

Fix

  • apis.mustache gates the explode loop on isMap, not isContainer. A bare type: object is isMap but not isContainer (only an additionalProperties map is), so it was assigned whole and the runtime bracketed it.
  • TypeScriptFetchClientCodegen copies isDeepObject. ExtendedCodegenParameter's copy constructor dropped it, so it was always false in the templates. A deepObject parameter is explode: true as well, and now stays assigned whole so the runtime brackets it.
  • as any on the indexing. A free-form object is typed object, and object[key] is TS7053 under strict.
  • A null entry is left out. The runtime sent {k: null} as k=null and {k: undefined} as k=undefined.
  • An entry is defined, not assigned. queryParameters['__proto__'] = v hits the prototype setter and drops the entry; Object.defineProperty keeps it (__proto__=x), as the runtime already does under deepObject.
  • Side effect: a deepObject parameter typed as a map was flattened on master (key=value); it now gets its brackets.

Verified

A client generated from the fixture, recording the URL it hands to fetch:

input on the wire
filter: {category: 'books', 'createdDate:gte': '2023-01-01'} category=books&createdDate%3Agte=2023-01-01
typedFilter: {category: 'books'} category=books
filter: {k: null, u: undefined, a: 'b', l: ['x', 2]} a=b&l=x&l=2 (before the null skip: k=null&u=undefined&a=b&l=x&l=2)
deepFilter: {category: 'books'} deepFilter%5Bcategory%5D=books (as on master)
flatFilter: {category: 'books'} flatFilter%5Bcategory%5D=books (as on master)

TypeScriptFetchClientCodegenTest#testExplodedObjectQueryParameter fails without the fix.

Known gaps

  • Models. A $refed object model is isModel, so it still goes on the wire whole. Its interface properties aren't the wire names (_float for float, a Date for dateTime), so a fix has to go through {{dataType}}ToJSON first.
  • explode: false is still bracketed; the spec wants flatFilter=category,books.
  • Name collisions. An entry goes into the same queryParameters object as the declared parameters, so an entry and a declared parameter with the same name overwrite each other, depending on emit order. Keeping both would mean merging at every query assignment in apis.mustache, not just this loop.
  • A null item of a list entry still goes out as null.

PR checklist


Summary by cubic

Fixes the typescript-fetch client so object query parameters with default form/explode go on the wire as one parameter per entry, keyed by the property name alone (category=books), instead of bracketed under the parameter name (filter[category]=books). deepObject and explode: false objects keep their previous wire format.

Bug Fixes

  • The explode branch was gated on isContainer, which free-form objects never set; it now keys on isMap, with an as any cast so strict TypeScript still compiles.
  • Null or undefined entries are now left off the wire instead of being sent as k=null / k=undefined.
  • ExtendedCodegenParameter's copy constructor dropped style-related flags, breaking deepObject parameters; it now copies isDeepObject, isFormStyle, isMatrix, isAllowEmptyValue, isSpaceDelimited, and isPipeDelimited. A duplicate this.isExplode = cp.isExplode; line from the latest commit should be removed.
  • Adds a test fixture covering the four style/explode combinations.

Known gap

  • A $refed object model as a query parameter still goes on the wire as a whole; fixing it requires routing through the model's ToJSON for wire-name translation, which is out of scope.

Written for commit df655b6. Summary will update on new commits.

Review in cubic


Generated with Claude Code

A query parameter whose schema is an object and whose style/explode are left at
their defaults — style: form, explode: true — must go on the wire as one
parameter per entry, keyed by the property name alone. The typescript-fetch
client bracketed it as filter[category]=books instead.

typescript-fetch/apis.mustache gated the explode branch on isContainer, which
DefaultCodegen.fromParameter only sets via updateParameterForMap, which in turn
needs ModelUtils.isMapSchema (so, additionalProperties). A bare type: object
instead goes through setTypeProperties, which sets isMap and isFreeFormObject
and leaves isContainer false. The parameter was therefore assigned whole and the
runtime's querystringSingleKey bracketed it. The branch is now keyed on isMap;
inside {{^isArray}}, isContainer implies isMap, so this is the same condition
plus free-form objects.

TypeScriptFetchClientCodegen's ExtendedCodegenParameter copy constructor copied
isExplode and style but not isDeepObject, isFormStyle, isMatrix,
isAllowEmptyValue, isSpaceDelimited or isPipeDelimited, so all six were false in
every typescript-fetch template regardless of the document. This is independent
of the template fix, and it is why fixing the template alone would have broken
deepObject parameters — they are explode: true too.

The loop body gains an `as any` cast on the indexing. It is not needed for a
declared map, but a free-form object is typed `object` and object[key] is
error TS7053 under strict, which is how most consumers compile.
@wiebren

wiebren commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Split per language as requested, out of #24797. This is the typescript-fetch half; the
siblings are:

The three touch disjoint sets of files under main/ and have no ordering dependency, so they
can be reviewed and merged independently. Each adds
modules/openapi-generator/src/test/resources/3_0/exploded-object-query-param.yaml at the
same path with identical content, so whichever lands first, the others rebase cleanly.

Each branch was tested on its own after the split, not just as part of the original combined
branch.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 7 files

Re-trigger cubic

@wiebren

wiebren commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

cubic came back clean on this PR, but its earlier run against the pre-split commit on #24797
raised two typescript-fetch points. Recording the dispositions here so they are not lost with
the stale review:

Declared object models still serialized whole — valid, pre-existing, deliberately out of
scope.
This is the known gap already in the description. cubic's suggested patch was to add
an isModel branch running the same Object.keys loop, and that would be actively wrong:
typescript-fetch interface properties are not the wire names, which is the whole reason the
ToJSON functions exist. FormatTest.ts maps 'float' to value['_float'] and
'pattern_with_digits' to value['patternWithDigits'], and dateTime needs
serializeDateTime. Iterating Object.keys over a model would put _float=… and a raw
Date on the wire — worse than the whole-object assignment it replaces. A correct fix routes
through {{dataType}}ToJSON first, which is a feature, not a template branch.

__proto__ as an own key is dropped — valid, exotic, not fixed here. Confirmed:

const src = JSON.parse('{"__proto__":"x","category":"books"}');
const queryParameters = {};
for (const key of Object.keys(src)) queryParameters[key] = src[key];
Object.keys(queryParameters);  // ['category'] — the __proto__ entry is gone

The assignment hits the prototype setter instead of creating an own property, so the entry
silently disappears. Worth noting this is not introduced here — the pre-existing
isContainer branch ran the same loop for declared maps; this PR only widens which
parameters reach it. Fixing it properly means giving queryParameters a null prototype or
using Object.defineProperty, which touches the shared runtime querystring path and every
generator that shares it. I would rather do that as its own PR than smuggle it into a
serialization fix. Say the word if you would prefer it here.

@wing328

wing328 commented Sep 1, 2026

Copy link
Copy Markdown
Member

thanks for the PR

cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04) @joscha (2024/10) @KannaKim (2026/07)

The runtime turned a null or undefined entry into k=null or k=undefined;
the loop now skips it, as the other ports do. Only isDeepObject is read
by the templates, so the copy constructor keeps that flag alone. The
mustache comment shrinks to one line, and the shared fixture's
description names the three combinations it covers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache">

<violation number="1" location="modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache:214">
P2: This assignment drops duplicate query names when an exploded object key collides with another parameter. Accumulate existing values into an array so the runtime emits both query occurrences.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

queryParameters[key] = requestParameters['{{paramName}}'][key];
const value = (requestParameters['{{paramName}}'] as any)[key];
if (value != null) {
queryParameters[key] = value;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This assignment drops duplicate query names when an exploded object key collides with another parameter. Accumulate existing values into an array so the runtime emits both query occurrences.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache, line 214:

<comment>This assignment drops duplicate query names when an exploded object key collides with another parameter. Accumulate existing values into an array so the runtime emits both query occurrences.</comment>

<file context>
@@ -202,14 +202,23 @@ export class {{classname}} extends runtime.BaseAPI {
-                queryParameters[key] = requestParameters['{{paramName}}'][key];
+                const value = (requestParameters['{{paramName}}'] as any)[key];
+                if (value != null) {
+                    queryParameters[key] = value;
+                }
             }
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a known gap, listed in the description: an exploded key equal to a declared parameter's name overwrites it, or is overwritten, depending on emit order. The runtime would repeat the key for an array value, but every declared query parameter is a plain queryParameters[name] = … assignment in apis.mustache, so keeping both means changing all of those sites to merge, not just this loop. That is a wider change than this PR; the kotlin port keeps both because its query map is multi-valued throughout.

Comment thread modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache Outdated
Assigning queryParameters['__proto__'] hits the Object.prototype setter
and drops the entry, while the runtime keeps such a key under deepObject
and in nested objects. The loop now defines the property instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 5 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

wiebren and others added 2 commits September 23, 2026 12:31
The codegen test cannot run the generated client, so its comment now
says it locks the code shape; the wire behaviour was checked separately.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>

@macjohnny macjohnny left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks for the fix

@macjohnny
macjohnny merged commit 2f1cfcf into OpenAPITools:master Sep 23, 2026
1 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants