-
Notifications
You must be signed in to change notification settings - Fork 39
Add default type for enum #302
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
Conversation
cda8b00
to
e7e0207
Compare
src/services/includers/batteries/openapi/generators/traverse.ts
Outdated
Show resolved
Hide resolved
if (value.type) { | ||
return value.type; | ||
} | ||
if (value.enum && Array.isArray(value.enum)) { |
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.
factor it out into separate function
isEnumOfImplementedType
/isEnumOfSupportedType
checking if value is one of the supported types
if so return that enum type, otherwise throwing
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.
fixed
@@ -234,3 +234,18 @@ function merge(value: OpenJSONSchemaDefinition): OpenJSONSchema { | |||
function isRequired(key: string, value: JSONSchema6): boolean { | |||
return value.required?.includes(key) ?? false; | |||
} | |||
|
|||
function inferType(value: OpenJSONSchema): JSONSchema6['type'] { |
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.
as far as i understand inferType
might return undefined
yet i can't find you handling that scenario
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.
This PR is about adding a type calculation for enum, this function does not handle undefined.
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.
This PR is about adding a type calculation for enum
-
you are adding function to infer type from value.
-
this function has undefined as one of its return values
-
either handle function returning undefined when you call it or throw error instead of returning undefined inside of the function.
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.
fixed
e7e0207
to
0a4a64a
Compare
@@ -20,6 +20,8 @@ const SPEC_RENDER_MODE_HIDDEN = 'hidden'; | |||
const SPEC_SECTION_NAME = 'Specification'; | |||
const SPEC_SECTION_TYPE = 'Open API'; | |||
|
|||
const SUPPORTED_ENUM_TYPES = ['string', 'number'] as const; |
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.
Please keep an array here for convenient type generation.
} | ||
|
||
function isSupportedEnumType(enumType: JsType): enumType is SupportedEnumType { | ||
return SUPPORTED_ENUM_TYPES.some((type) => enumType === type); |
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 includes method does not allow using TS
0a4a64a
to
b989c29
Compare
I made a function for calculating the enum type: