orval
orval copied to clipboard
Add option to generate union instead of enum
Related
- https://github.com/orval-labs/orval/issues/907
Problem
I've noticed I don't use the generated enums in schemas.
I use only the generated type.
I assume there are other users-devs who also need ability to disable enum/const generation.
So I propose to add a flag override.disableEnumGeneration, related to override.useNativeEnums
cc @AllieJonsson @soartec-lab ?
Im not sure I understand the use case. Why are you not using the enum? How is your schema structured like?
If we just remove the enum, what should we do with the property foo in this case below?
export const MyEnum {
a: "A"
} as const;
export type MyObject {
foo: MyEnum;
}
If we just remove the enum, what should we do with the property foo in this case below?
This is a real world example of what Orval generates:
/**
* Order status:
- `24` (new)
- `21` (review)
*/
export type StatusOrder = typeof StatusOrder[keyof typeof StatusOrder];
// eslint-disable-next-line @typescript-eslint/no-redeclare
export const StatusOrder = {
NUMBER_24: '24',
NUMBER_21: '21',
} as const;
Above, you can see StatusOrder is declared twice, I only need the type, I don't need the unreadable const StatusOrder that I don't use.
So the preferred generated result would be:
/**
* Order status:
- `24` (new)
- `21` (review)
*/
export type StatusOrder = "24" | "21"
OpenAPI schema for the example above:
{
"components": {
"schemas": {
"status_order": {
"type": "string",
"enum": ["24", "21"],
"description": "Order status:\n- `24` (new)\n- `21` (review)"
}
}
}
}
Why are you not using the enum?
Because we generate OpenAPI docs from TypeScript. We already have an enum (code example below).
We don't have SDK and typings, that's what we use orval for.
The code below generates the OpenAPI snippet above.
export const order = {
new: `24`,
review: `21`,
} as const satisfies Record<string, `${number}`>
Ah i see, so you would like to be able to create a union type instead of enums? That makes sense!
@soartec-lab maybe we should make useNativeEnums obsolete and make a new setting, something like
enums:
| "const" // as todays default
| "enum" // same as useNativeEnums=true
| "union" // this new way
@o-alexandrov @AllieJonsson
That's good! This is going to be a breaking change, right? If so, could you please comment here with a sample of the output before and after the change?
Im thinking we could leave useNativeEnums in the setting now, but displaying a message that it is deprecated if it is used. Then when we normalize the settings we can convert useNativeEnuns to our new setting
@AllieJonsson
However, if you output it using the new method, you won't be able to access it as StatusOrder.NUMBER_24, right?
Yes, if you generate using the new suggested option the no. If you want to be able to do that you set enums to "const" or "enum" . We should have enum be "const" as default, to have the same behavior as today.
I still think we should support useNativeEnums for a few releases (maybe until version 8.0?), to allow users to migrate to the new option.
Here is my suggestion:
// types.ts
export type OverrideOutput = {
...
/**
* @deprecated use enumGeneration: "enum" instead
*/
useNativeEnums?: boolean;
enumGeneration?: 'const' | 'enum' | 'union';
}
export type NormalizedOverrideOutput = {
...
enumGeneration: 'const' | 'enum' | 'union';
}
// options.ts
export const normalizeOptions = async (...) => {
...
if (outputOptions.override?.useNativeEnums !== undefined) {
log("'useNativeEnums' is deprecated, use 'enumGeneration: \"enum\"' instead.");
}
const normalizedOptions: NormalizedOptions = {
...
output: {
...
override: {
...
enumGeneration: !!outputOptions.override?.useNativeEnums ? 'enum' : (outputOptions.override?.enumGeneration ?? 'const'),
}
}
}
}
@AllieJonsson
I see, so does this mean that enumGeneration has the following types?
const: Current default behavior (useNativeEnums: false)enum: WhenuseNativeEnums: trueunion: A simple union as proposed in this issue
@soartec-lab Yes, that is my proposal!
Oh, that makes sense.
I haven't decided how long we'll support useNativeEnums, but I'm in favor of adding a new option for now.
In that case, I think the name should include the ability to control the type, like enum GenerateType.
@o-alexandrov @AllieJonsson
Would you like to submit a PR for this?
Sure, Ill fix this tomorrow morning!
You're great! i change assignment to you.
Can we change the title of this issue to something along the lines of
Add option to generate union instead of enum? 😊
@AllieJonsson @soartec-lab Thank you, it works great, except for one case below.
Could you please reopen the issue, as enumGenerationType: "union" currently results in a bug (at the latest commit on master branch).
Generated code (containing a bug; as FollowsComputeOnBackend, ValueToRemoveAnAttribute JavaScript variables don't exist):
// eslint-disable-next-line @typescript-eslint/no-redeclare
export const PatchV1UserRelationshipIdBodyFollows = {...FollowsComputeOnBackend,...ValueToRemoveAnAttribute,} as const
// eslint-disable-next-line @typescript-eslint/no-redeclare
export const PatchV1UserRelationshipIdBodyBlocks = {...BlocksComputeOnBackend,...ValueToRemoveAnAttribute,} as const
export type PatchV1UserRelationshipIdBody = {
follows?: typeof PatchV1UserRelationshipIdBodyFollows[keyof typeof PatchV1UserRelationshipIdBodyFollows] ;
blocks?: typeof PatchV1UserRelationshipIdBodyBlocks[keyof typeof PatchV1UserRelationshipIdBodyBlocks] ;
};
Probably, the getEnum:
https://github.com/orval-labs/orval/blob/db15ab61f01e85bca1de3b1af1073aff0ecd1af4/packages/core/src/getters/enum.ts#L14
Should be used here:
- that function isn't easily readable and I couldn't fix the issue right away, so I'm commenting here asking for your guidance as you probably know the related code
https://github.com/orval-labs/orval/blob/db15ab61f01e85bca1de3b1af1073aff0ecd1af4/packages/core/src/getters/combine.ts#L194
Example OpenAPI spec (this is a slightly simplified version to easily reproduce the issue):
paths/example-path: {
"patch": {
"requestBody": {
"required": true,
"content": {
"application/json": {
"schema": {
"additionalProperties": false,
"type": "object",
"properties": {
"follows": {
"anyOf": [
{
"type": "string",
"enum": ["example-true"]
},
{
"type": "string",
"enum": ["example-false"]
}
]
},
},
"required": []
}
}
}
},
"responses": {
"200": {
"description": "Example OK"
},
},
}
}
bump