-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[typescript-fetch] Fix ToJSON for non-descriminator oneOf constructs #12513
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 ToJSON for non-descriminator oneOf constructs #12513
Conversation
macjohnny
left a comment
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.
thanks for your contribution!
modules/openapi-generator/src/main/resources/typescript-fetch/modelGeneric.mustache
Outdated
Show resolved
Hide resolved
99582b9 to
10dceb1
Compare
10dceb1 to
161bb36
Compare
macjohnny
left a comment
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.
LGTM
|
It would be great to get this merged into the next patch release :) How does the merge flow in this project look like? Do you use merge windows? |
|
|
||
| {{^discriminator}} | ||
| return { {{#oneOf}}...{{{.}}}ToJSON(value){{^-last}}, {{/-last}}{{/oneOf}} }; | ||
| {{#oneOf}} | ||
| if (instanceOf{{{.}}}(value)) { | ||
| return {{{.}}}ToJSON(value as {{{.}}}); | ||
| } | ||
| {{/oneOf}} | ||
|
|
||
| return {}; | ||
| {{/discriminator}} |
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.
I'm not entirely familiar with this code, but I think something similar should also be added to modelGeneric.mustache.
In a project of mine where I use allOf for inheritance. The base class has a {class}FromJSONTyped function that handles all the discriminator cases, but {class}ToJSON doesn't include any discriminator code at all and just returns an object with properties of the base class.
I'm currently patching the generated code to add ignoreDiscriminator to the ToJSON functions which will be set to true when invoked from the child class's ToJSON, if false ToJSON in the base class calls the respective child class ToJSON based on the discriminator.
Here are the relevant models of my openapi config:
"Activity": {
"required": ["$type", "id", "title"],
"type": "object",
"properties": {
"$type": {
"$ref": "#/components/schemas/ActivityType"
},
"id": {
"type": "string"
},
"title": {
"type": "string"
},
},
"additionalProperties": false,
"discriminator": {
"propertyName": "$type",
"mapping": {
"exercise": "#/components/schemas/ActivityExercise",
"job": "#/components/schemas/ActivityJob",
}
}
},
"ActivityExercise": {
"required": ["$type", "category"],
"type": "object",
"allOf": [
{
"$ref": "#/components/schemas/Activity"
}
],
"properties": {
"$type": {
"$ref": "#/components/schemas/ActivityType"
},
"category": {
"$ref": "#/components/schemas/ActivityCategory"
},
"muscleShares": {
"type": "array",
"items": {
"$ref": "#/components/schemas/MuscleShare"
},
"nullable": true
}
},
"additionalProperties": false
},
"ActivityJob": {
"required": ["$type"],
"type": "object",
"allOf": [
{
"$ref": "#/components/schemas/Activity"
}
],
"properties": {
"$type": {
"$ref": "#/components/schemas/ActivityType"
},
"muscleShares": {
"type": "array",
"items": {
"$ref": "#/components/schemas/MuscleShare"
},
"nullable": true
}
},
"additionalProperties": false
},
This PR fixes #12510
Previously, the generated code tried to create an
anyobject with all possible variants of theoneOfconstruct, if no descriminator was specified. I couldn't find out how to use a descriminator though, as this doesn't seem to be documented in the openapi standard.Anyhow, the old behavior didn't make a lot of sense and the compiler also failed to build this code.
As a fix, the newly generated code now only returns the result of the
ToJSONfunction for the given variant.I researched how to check for identity during runtime in TS, and it seems that the only way to do this is via custom typeguards functions?
Hence, I created a typeguard function for each interface in the style of
instanceOf{{classname}}.The PR is split into two commits:
I'm not sure if I'm supposed to ping the whole TS team, but the checklist talks about "members", so here we go 😅 :
@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir
@petejohansonxo @amakhrov @davidgamero @mkusaka
Sorry if that wasn't correct.
PR checklist
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/configs/java*.For Windows users, please run the script in Git BASH.
master(6.0.1) (patch release),6.1.x(breaking changes with fallbacks),7.0.x(breaking changes without fallbacks)