[CHK-1807] feat: allow object properties to be defined with oneOf#322
[CHK-1807] feat: allow object properties to be defined with oneOf#322giovanniberti wants to merge 10 commits intomasterfrom
oneOf#322Conversation
Jira Pull Request LinkThis Pull Request refers to the following Jira issue CHK-1807 |
467e78e to
215ccde
Compare
215ccde to
72934fa
Compare
gquadrati
left a comment
There was a problem hiding this comment.
Just some small comments
| export const {{ definitionName }} = | ||
| t.union([ | ||
| {% for schema in prop -%} | ||
| {% if schema.type == "object" or schema.type == "array" %} |
There was a problem hiding this comment.
This is covered by the snapshot (specifically the generated constants named OneOfPropertyTestFieldsN like here)
Would you suggest a separate test case?
There was a problem hiding this comment.
What I see from generated code is:
export const OneOfPropertyTestFields4 = t.readonlyArray(
t.object,
"array of object"
);
Inline array object definitions are not supported, unfortunately. :(
| it("should generate an object with a union property when that property has oneOf", async () => { | ||
| const definitonName = "OneOfPropertyTest"; | ||
| const definition = getDefinitionOrFail(spec, definitonName); | ||
|
|
||
| const code = await renderDefinitionCode( | ||
| definitonName, | ||
| getParser(spec).parseDefinition( | ||
| // @ts-ignore | ||
| definition | ||
| ), | ||
| false | ||
| ); | ||
|
|
||
| expect(code).toContain("t.union"); | ||
| expect(code).toMatchSnapshot("oneof-property-test"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Consider to add some tests in /e2e/src/tests/test-api-v3/definitions.test.ts and /e2e/src/tests/test-api/definitions.test.ts
There was a problem hiding this comment.
Amazing, thanks! In addition to that, could you please include some scenarios where we expect things to go wrong? For example:
const variant5 = {
fields: [{ name: 42 }]
};
Description
oneOfwhen defining object propertiesMotivation and Context
This PR implements defining object properties with
oneOf.Property definition with
oneOfis permitted by the OpenAPI spec but currently fails with an error during generation (note that this is a different case from a separate schema defined withoneOf).Note that this PR:
discriminator(which has more requirements, for example it cannot be used with inline schemas). Code generation doesn't fail withdiscriminatorbut no schema checking is made and no special type information about the discriminator is propagated to the generated code.allOforanyOf(should we add it in this PR or in another one? I don't like very much supportingoneOffor properties but not the others)How Has This Been Tested?
Manual inspection of generated snapshots used for testing
Screenshots (if appropriate):
Types of changes
Checklist: