-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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] correct oneOf model to or
not concat
when handling enums
#10017
Closed
Closed
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
505ceaa
[typescript-fetch] correct one of model
prince-chrismc eee43b4
following build steps
39b6299
following build steps
cf1b7b1
Update modelOneOf.mustache
prince-chrismc 59c117e
following build steps
9df3375
missing file
200a6c0
exmaple with bad generation
45b537a
revert bad change
3503ef7
proposed work around
b45cf8f
make sure one of works with falsy
4b40ca2
test object enum
2f5fb8d
Revert "test object enum"
e68fe47
following build steps
bf89f40
following build steps
5f90822
Update one-of.yaml
prince-chrismc 27abd20
following build steps
3fb4b52
better null/truthy handling
prince-chrismc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
generatorName: typescript-fetch | ||
outputDir: samples/client/petstore/typescript-fetch/builds/one-of | ||
inputSpec: modules/openapi-generator/src/test/resources/3_0/typescript-fetch/one-of.yaml |
10 changes: 8 additions & 2 deletions
10
modules/openapi-generator/src/main/resources/typescript-fetch/modelEnum.mustache
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,16 @@ | ||
{{>modelEnumInterfaces}} | ||
|
||
export function {{classname}}FromJSON(json: any): {{classname}} { | ||
export function {{classname}}FromJSON(json: any): {{classname}} | null { | ||
if ((json === undefined) || (json === null)) { | ||
return json; | ||
} | ||
if(!Object.values({{classname}}).includes(json as {{classname}})) { | ||
return null; | ||
} | ||
Comment on lines
+4
to
+9
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. Are these runtime checks allowed? |
||
return {{classname}}FromJSONTyped(json, false); | ||
} | ||
|
||
export function {{classname}}FromJSONTyped(json: any, ignoreDiscriminator: boolean): {{classname}} { | ||
export function {{classname}}FromJSONTyped(json: any, ignoreDiscriminator: boolean): {{classname}} | null { | ||
return json as {{classname}}; | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55 changes: 55 additions & 0 deletions
55
modules/openapi-generator/src/test/resources/3_0/typescript-fetch/one-of.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
openapi: 3.0.0 | ||
info: | ||
version: 1.0.0 | ||
title: Enum test | ||
servers: | ||
- url: http://localhost:3000 | ||
paths: | ||
/fake/enum-request-ref: | ||
get: | ||
operationId: fake-enum-request-get-ref | ||
responses: | ||
200: | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: '#/components/schemas/PatternObject' | ||
post: | ||
operationId: fake-enum-request-post-ref | ||
responses: | ||
200: | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: '#/components/schemas/PatternObject' | ||
requestBody: | ||
content: | ||
application/json: | ||
schema: | ||
$ref: "#/components/schemas/PatternObject" | ||
components: | ||
schemas: | ||
PatternObject: | ||
type: object | ||
properties: | ||
string: | ||
type: string | ||
nullable-string: | ||
nullable: true | ||
type: string | ||
number-enum: | ||
type: number | ||
PatternObjectTwo: | ||
type: object | ||
properties: | ||
boolean: | ||
type: boolean | ||
ObjectCombinedOneOf: | ||
type: "object" | ||
properties: | ||
combined-string-enum: | ||
oneOf: | ||
- $ref: '#/components/schemas/PatternObject' | ||
- $ref: '#/components/schemas/PatternObjectTwo' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
samples/client/petstore/typescript-fetch/builds/enum/models/NumberEnumTwo.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* tslint:disable */ | ||
/* eslint-disable */ | ||
/** | ||
* Enum test | ||
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) | ||
* | ||
* The version of the OpenAPI document: 1.0.0 | ||
* | ||
* | ||
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). | ||
* https://openapi-generator.tech | ||
* Do not edit the class manually. | ||
*/ | ||
|
||
/** | ||
* | ||
* @export | ||
* @enum {string} | ||
*/ | ||
export enum NumberEnumTwo { | ||
NUMBER_4 = 4, | ||
NUMBER_5 = 5, | ||
NUMBER_6 = 6 | ||
} | ||
|
||
export function NumberEnumTwoFromJSON(json: any): NumberEnumTwo | null { | ||
if ((json === undefined) || (json === null)) { | ||
return json; | ||
} | ||
if(!Object.values(NumberEnumTwo).includes(json as NumberEnumTwo)) { | ||
return null; | ||
} | ||
return NumberEnumTwoFromJSONTyped(json, false); | ||
} | ||
|
||
export function NumberEnumTwoFromJSONTyped(json: any, ignoreDiscriminator: boolean): NumberEnumTwo | null { | ||
return json as NumberEnumTwo; | ||
} | ||
|
||
export function NumberEnumTwoToJSON(value?: NumberEnumTwo | null): any { | ||
return value as any; | ||
} | ||
|
51 changes: 51 additions & 0 deletions
51
samples/client/petstore/typescript-fetch/builds/enum/models/NumberOneOf.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* tslint:disable */ | ||
/* eslint-disable */ | ||
/** | ||
* Enum test | ||
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator) | ||
* | ||
* The version of the OpenAPI document: 1.0.0 | ||
* | ||
* | ||
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). | ||
* https://openapi-generator.tech | ||
* Do not edit the class manually. | ||
*/ | ||
|
||
import { | ||
NumberEnum, | ||
NumberEnumTwo, | ||
NumberEnumFromJSONTyped, | ||
NumberEnumToJSON, | ||
NumberEnumTwoFromJSONTyped, | ||
NumberEnumTwoToJSON, | ||
} from './'; | ||
|
||
/** | ||
* @type NumberOneOf | ||
* | ||
* @export | ||
*/ | ||
export type NumberOneOf = NumberEnum | NumberEnumTwo; | ||
|
||
export function NumberOneOfFromJSON(json: any): NumberOneOf { | ||
return NumberOneOfFromJSONTyped(json, false); | ||
} | ||
|
||
export function NumberOneOfFromJSONTyped(json: any, ignoreDiscriminator: boolean): NumberOneOf { | ||
if ((json === undefined) || (json === null)) { | ||
return json; | ||
} | ||
return NumberEnumFromJSONTyped(json, true) || NumberEnumTwoFromJSONTyped(json, true); | ||
} | ||
|
||
export function NumberOneOfToJSON(value?: NumberOneOf | null): any { | ||
if (value === undefined) { | ||
return undefined; | ||
} | ||
if (value === null) { | ||
return null; | ||
} | ||
return NumberEnumToJSON(value as NumberEnum) ?? NumberEnumTwoToJSON(value as NumberEnumTwo); | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
this is gonna be a breaking change. if an enum property is marked as required / non-nullable in the spec, all consumers expect that it's defined.
after this change some apps will likely stop compiling.
why do yo need it anyway? you can keep the original code here - return the enum value as is, without any processing
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.
Typescript gave a me a warning about the implicit return null. I believe it can be removed
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.
that's because this PR changed the code inside the function, narrowing down
json
type tonull
before returning itThere 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.
Playing around with this more... it previously returned a value that was not valid. so if you passed
7
and your enum was[0, 1, 2]
you would get back 7 which is not the right type.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.
well, we trust the api response matches the spec. otherwise the code is gonna break anyway, since we don't do full shape verification
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.
The challenge is older clients being used with newer backends which just weren't aware of the existence of such values
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 would say,
null
is as unexpected as a bogus enum value (if the property is not marked as nullable).Again, i honestly believe it's way out of scope of the generated api client.