fix: Don't apply spread operator to strings#18185
fix: Don't apply spread operator to strings#18185macjohnny merged 1 commit intoOpenAPITools:masterfrom
Conversation
|
@macjohnny The PR I submitted yesterday had an issue so I'm creating this one to address it. |
|
@giovannicimolin I don't think this will have the intended effect of fixing only string enums without breaking the interfaces. The previous code would object assign all relevant interfaces which meant that the resulting object would satisfy constraints for any of them, but the new solution will be "truthy" for the first I'm not sure what the solution is to fix for this but I'm fairly sure this will break folks with "oneOf" w/ objects and not strings/enums |
|
@rickyrombo Hey! Thanks for the feedback here. I'm not super experienced in the OpenAPITools, so my knowledge was (and still is) limited here. That said, I can work on a fix for this! Could you give me more details and maybe an API scheme that would break? |
|
in #18702, the behavior of the oneOf case without discriminator was changed to use the instanceOfXXX functions. I guess without the discriminator, its just hard to correctly infer the type/apply the right transforms |
I made some incorrect assumptions on #18154 and as a result the generator ends up applying the spread operator on a string, which is the wrong behavior.
This PR fixes that replacing the spread operation with a
||. This makes the API client have the correct behavior and properly pass the Enum value forward with the correct type.This doesn't fix the case when
discriminatoris being used, and I don't know the use case of that or how it works, so I won't touch that code.I'm not too familiar with the internals, so some guidance in adding tests for this would be a nice extra for this PR.
Testing instructions
I tested this using the APIs from my project.
Relevant details:
oneOfmerging multiple Enums (example on fix: Fix schema generation foroneOfwhen using TS-FETCH client #18154).PR checklist
Commit all changed files.
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*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming 7.1.0 minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)