Skip to content
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] Generate oneOf schemas as type unions #19494

Merged
merged 19 commits into from
Aug 30, 2024

Conversation

joscha
Copy link
Contributor

@joscha joscha commented Aug 30, 2024

#19027 + latest master + revert of #19481, see #19027 (comment)

reverts #19481
closes #19027

@joscha joscha changed the title feature/typescriptFixOneOf [Typescript] Generate oneOf schemas as type unions Aug 30, 2024
@macjohnny macjohnny merged commit 7510e6b into OpenAPITools:master Aug 30, 2024
15 checks passed
@joscha joscha deleted the feature/typescriptFixOneOf branch August 30, 2024 14:26
@wing328 wing328 added this to the 7.9.0 milestone Oct 7, 2024
@simon-abbott
Copy link
Contributor

simon-abbott commented Oct 14, 2024

This change breaks allOf unions, they don't generate discriminator information at all.

EDIT: Opened an issue for this: #19868.

@joscha
Copy link
Contributor Author

joscha commented Oct 15, 2024

@ksvirkou-hubspot would you mind having a look at #19868 by any chance, please?

@joscha
Copy link
Contributor Author

joscha commented Oct 19, 2024

@simon-abbott can you produce a pull request with a fixture that shows the breakage or link me to some code showing what it should be vs. what it is right now? Maybe we can work together to try and solve this one?

@ksvirkou-hubspot
Copy link
Contributor

ksvirkou-hubspot commented Oct 22, 2024

@ksvirkou-hubspot would you mind having a look at #19868 by any chance, please?

This issue occurs because the schema described doesn't include a discriminator. Without a discriminator, it's challenging to find out which object the API returns. I'm uncertain about the best course of action in this scenario. I could either throw an exception or return a JSON object.
Currently, I'm reviewing other generators to understand their approach to this situation.
Do you have any idea?

@simon-abbott
Copy link
Contributor

simon-abbott commented Oct 22, 2024

@simon-abbott can you produce a pull request with a fixture that shows the breakage or link me to some code showing what it should be vs. what it is right now? Maybe we can work together to try and solve this one?

Unfortunately I don't have time to make a full MRE at the moment, but If you generate the example schema in #19868 in 7.7.0 and again in 7.9.0 (I didn't test in 7.8, but I predict it will behave like 7.7) you can see the difference in the main Fruit model. The main cause is that previously the Fruit model would be generated as a franken-class containing the properties (and thus the getAttributeTypeMap) of the constituent parts. After this PR the main Fruit model is a super-thin wrapper.

This issue occurs because the schema described doesn't include a discriminator. [...] I'm uncertain about the best course of action in this scenario. [...]

See my reply in #19868. The spec says what to do.

@ksvirkou-hubspot
Copy link
Contributor

@joscha let's discuss it in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants