-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[bugfix] [typescript-fetch] add oneOf string union & array support (#19909) #20193
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -15,9 +15,12 @@ | |||||
|
||||||
import * as runtime from '../runtime'; | ||||||
import type { | ||||||
TestArrayResponse, | ||||||
TestResponse, | ||||||
} from '../models/index'; | ||||||
import { | ||||||
TestArrayResponseFromJSON, | ||||||
TestArrayResponseToJSON, | ||||||
TestResponseFromJSON, | ||||||
TestResponseToJSON, | ||||||
} from '../models/index'; | ||||||
|
@@ -51,4 +54,28 @@ export class DefaultApi extends runtime.BaseAPI { | |||||
return await response.value(); | ||||||
} | ||||||
|
||||||
/** | ||||||
*/ | ||||||
async testArrayRaw(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise<runtime.ApiResponse<TestArrayResponse>> { | ||||||
const queryParameters: any = {}; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this be at least:
Suggested change
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, in fact, it can even be runtime.HTTPQuery But i think this is outside of the scope of this PR as I did not make any changes to the api mustache, i just added a new testcase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, sorry, I assumed because it was a newly generated block, it originated from your change, too. |
||||||
|
||||||
const headerParameters: runtime.HTTPHeaders = {}; | ||||||
|
||||||
const response = await this.request({ | ||||||
path: `/test-array`, | ||||||
method: 'GET', | ||||||
headers: headerParameters, | ||||||
query: queryParameters, | ||||||
}, initOverrides); | ||||||
|
||||||
return new runtime.JSONApiResponse(response, (jsonValue) => TestArrayResponseFromJSON(jsonValue)); | ||||||
} | ||||||
|
||||||
/** | ||||||
*/ | ||||||
async testArray(initOverrides?: RequestInit | runtime.InitOverrideFunction): Promise<TestArrayResponse> { | ||||||
const response = await this.testArrayRaw(initOverrides); | ||||||
return await response.value(); | ||||||
} | ||||||
|
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that the same as https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java#L1123 ?
if overriding
toOneOfName
won't meet your requirement, can you please elaborate?cc @OpenAPITools/generator-core-team
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wing328 Yea, they are pretty similar. The core difference is that
toOneOfName
seems to build the whole oneOf type, while mine only builds the string literal part. It would be nice to refactor them both into one method.The Problem is,
toOneOfName
seems to be completely unused in the oneOf model generation.Currently, the different oneOf union parts are just put in an array and are appended together inside the mustache, not using
toOneOfName
at all.Refactoring it to build the full oneOf type inside the
toOneOfName
instead of inside the mustache seems to be a major change to how the code generation works and i need some input if this is the correct way, or if i misunderstood something, as i am new to the codebase