-
-
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] handle uniqueItems correctly in model and api #8695
[typescript-fetch] handle uniqueItems correctly in model and api #8695
Conversation
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
uniqueItems produces Set instead of Array as type, but the corresponding code did not treat Set correctly use `Array.from` and `new Set` to convert from Set to Array and vice versa, if uniqueItems is true. related to OpenAPITools#8258, but only fixes typescript-fetch
b202b3c
to
531a852
Compare
@t-h-e thanks for your contribution. please follow the steps described in https://github.com/OpenAPITools/openapi-generator/blob/master/.github/PULL_REQUEST_TEMPLATE.md |
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 code looks good, but I think this might be a breaking change if currently someone uses a type-mapping. I am not sure how we should proceed. @wing328 any thoughts?
@macjohnny Thanks for the hint. I updated the samples. As @macjohnny mentioned correctly, this change will break setups, where type-mapping has been used. I think I found another issue, unless I'm mistaken. Pet.ts in samples/client/petstore/typescript-fetch/builds/default-3.0/models may not work correctly in line 94 and 112, as photoUrls of type Set is treated as primitive. This worked when photoUrls was of type Array, but not with Set. |
For other generators, we do not consider it as a breaking change as we're making a correction to the output. Let's see if any TS users have a different opinion on this. |
@t-h-e thanks for the PR, which has been included in the v5.1.0 release: https://twitter.com/oas_generator/status/1373636654024380423 |
uniqueItems produces Set instead of Array as type, but the corresponding code did not treat Set correctly
use
Array.from
andnew Set
to convert from Set to Array and vice versa, if uniqueItems is true.related to #8258, but only fixes typescript-fetch