-
Notifications
You must be signed in to change notification settings - Fork 10
[CHK-1807] feat: allow object properties to be defined with oneOf
#322
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
787d76a
9909b3f
c632e72
96db5e6
d376845
adfa486
72934fa
f41a08f
b768f81
4c70dee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -265,6 +265,15 @@ | |
| {{ defineConst(definition.default, definitionName, typedef, false, inline) }} | ||
| {% endmacro %} | ||
|
|
||
| {## | ||
| # defines a oneOf property | ||
| #} | ||
| {% macro defineOneOfProperty(parentPropName, propName, definition, inline = false) -%} | ||
| {{- 'import * as t from "io-ts";' | addImport -}} | ||
| {% set definitionName %}{{ parentPropName }}{{ propName | capitalizeFirst }}{% endset %} | ||
| {{ definitionName }} | ||
|
giovanniberti marked this conversation as resolved.
Outdated
|
||
| {% endmacro %} | ||
|
|
||
| {## | ||
| # defines an object property of some prop.type | ||
| #} | ||
|
|
@@ -290,6 +299,8 @@ | |
| {{ defineString(propName, prop, true) }} | ||
| {% elif prop.type == "boolean" %} | ||
| {{ defineBoolean(propName, prop, true) }} | ||
| {% elif prop.oneOf %} | ||
| {{ defineOneOfProperty(parentPropName, propName, prop.oneOf, true)}} | ||
| {% else %} | ||
| // TODO: generate model for definition "{{ propName }}: {{ prop.type }}" | ||
| {% endif %} | ||
|
|
@@ -323,6 +334,38 @@ | |
| {% endfor %} | ||
| {% endmacro -%} | ||
|
|
||
| {% macro defineOneOf(definitionName, prop, strictInterfaces, camelCasedPropNames) -%} | ||
| {{- 'import * as t from "io-ts";' | addImport -}} | ||
|
|
||
| {% for schema in prop -%} | ||
| {% if schema.type == "object" %} | ||
| {% set name %}{{ definitionName }}{{ loop.index }}{% endset %} | ||
| {{ defineObject(name, schema, strictInterfaces, camelCasedPropNames) }} | ||
| {% elif schema.$ref %} | ||
| {%- set realPropName = schema.$ref | splitBy("/") | last -%} | ||
| {{ importLocalProp(realPropName) }} | ||
| {% elif schema.type == "array" %} | ||
| {% set name %}{{ definitionName }}{{ loop.index }}{% endset %} | ||
| {{ defineArray(name, schema) }} | ||
| {% endif %} | ||
|
giovanniberti marked this conversation as resolved.
|
||
| {% endfor %} | ||
|
|
||
| export const {{ definitionName }} = | ||
| t.union([ | ||
| {% for schema in prop -%} | ||
| {% if schema.type == "object" or schema.type == "array" %} | ||
|
Contributor
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. ditto
Author
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. This is covered by the snapshot (specifically the generated constants named Would you suggest a separate test case?
Contributor
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. What I see from generated code is: Inline array object definitions are not supported, unfortunately. :( |
||
| {{ definitionName }}{{ loop.index }}, | ||
| {% elif schema.$ref %} | ||
| {{ schema.$ref | splitBy("/") | last }}, | ||
| {% endif %} | ||
| {% endfor %} | ||
| ], | ||
| "{{ definitionName }}" | ||
| ); | ||
|
|
||
| export type {{ definitionName }} = t.TypeOf<typeof {{ definitionName }}>; | ||
| {% endmacro -%} | ||
|
|
||
| {## | ||
| # define object properties recursively, | ||
| # supports additionaProperties, allOf and oneOf. | ||
|
|
@@ -341,6 +384,9 @@ | |
| {% if prop.type == "object" %} | ||
| {% set composedPropName %}{{ definitionName }}{{ propName | capitalizeFirst }}{% endset %} | ||
| {{ defineObject(composedPropName, prop, strictInterfaces, camelCasedPropNames) }} | ||
| {% elif prop.oneOf %} | ||
| {% set composedPropName %}{{ definitionName }}{{ propName | capitalizeFirst }}{% endset %} | ||
| {{ defineOneOf(composedPropName, prop.oneOf, strictInterfaces, camelCasedPropNames) }} | ||
| {% endif %} | ||
| {% endfor %} | ||
|
|
||
|
|
@@ -418,31 +464,7 @@ | |
| {% elif definition.oneOf %} | ||
| {% set oneOfProps = definition.oneOf if definition.oneOf else definition.allOf %} | ||
|
|
||
| {{- 'import * as t from "io-ts";' | addImport -}} | ||
|
|
||
| {% for schema in oneOfProps -%} | ||
| {% if schema.type == "object" %} | ||
| {{ defineObject(definitionName + loop.index, schema, strictInterfaces, camelCasedPropNames) }} | ||
| {% elif schema.$ref %} | ||
| {%- set realPropName = schema.$ref | splitBy("/") | last -%} | ||
| {{ importLocalProp(realPropName) }} | ||
| {% endif %} | ||
| {% endfor %} | ||
|
|
||
| export const {{ definitionName }} = | ||
| t.union([ | ||
| {% for schema in oneOfProps -%} | ||
| {% if schema.type == "object" %} | ||
| {{ definitionName + loop.index }}, | ||
| {% elif schema.$ref %} | ||
| {{ schema.$ref | splitBy("/") | last }}, | ||
| {% endif %} | ||
| {% endfor %} | ||
| ], | ||
| "{{ definitionName }}" | ||
| ); | ||
|
|
||
| export type {{ definitionName }} = t.TypeOf<typeof {{ definitionName }}>; | ||
| {{ defineOneOf(definitionName, oneOfProps, strictInterfaces, camelCasedPropNames) }} | ||
|
|
||
| {% elif definition.type == "number" %} | ||
|
|
||
|
|
||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests in 4c70dee
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.
Amazing, thanks! In addition to that, could you please include some scenarios where we expect things to go wrong? For example: