-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[typescript-fetch] fix double parsing deep object parameters #6706
[typescript-fetch] fix double parsing deep object parameters #6706
Conversation
@macjohnny thanks for the heads-up, I'll write some unit tests to make sure the feature doesn't regress, or break because of other side effects |
@Vrtak-CZ thanks for the patch, but doesn't it mean that a deepobject parameter is now never escaped after your change? |
Looks like my change was done in 4.x and something must have changed in 5.x which changed behavior. I can reproduce the double escaping with today's master branch. @macjohnny is there any way to test the generated TypeScript code? E.g. do a So here's what I got so far, a dummy yaml: openapi: 3.0.0
info:
title: Sample API
description: Optional multiline or single-line description in [CommonMark](http://commonmark.org/help/) or HTML.
version: 0.1.9
servers:
- url: http://api.example.com/v1
description: Optional server description, e.g. Main (production) server
- url: http://staging-api.example.com
description: Optional server description, e.g. Internal staging server for testing
paths:
/users:
get:
summary: Returns a list of users.
description: Optional extended description in CommonMark or HTML.
parameters:
- in: query
name: someDeepObject
style: deepObject
required: true
schema:
type: object
responses:
'200': # status code
description: A JSON array of user names
content:
application/json:
schema:
type: array
items:
type: string Unfortunately, when I apply @Vrtak-CZ 's patch, I'm getting:
e.g. we cannot just assign an object to So let's gather some opinions - fix it by extending |
@haraldF thanks for testing.
that's what we do too because of this. But I do not know better solution and also there is already handler for object inside of |
Thanks for the info and sorry for the delay. I did some more digging and the |
closing this one in favor of #6860 |
Fixing double parsing query parameters for deep object.
Currently it generates something like this:
It's because there is double parsing one of it is what i removing and another one here:
openapi-generator/modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache
Lines 209 to 226 in ca3a23b
openapi-generator/modules/openapi-generator/src/main/resources/typescript-fetch/runtime.mustache
Lines 219 to 221 in ca3a23b
PR checklist
./bin/generate-samples.sh
to update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example./bin/generate-samples.sh bin/config/java*
. For Windows users, please run the script in Git BASH.master
@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov