Skip to content

Add option to generate union instead of enum #2010

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

Open
o-alexandrov opened this issue Apr 7, 2025 · 16 comments · Fixed by #2018 · May be fixed by #2057
Open

Add option to generate union instead of enum #2010

o-alexandrov opened this issue Apr 7, 2025 · 16 comments · Fixed by #2018 · May be fixed by #2057
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@o-alexandrov
Copy link
Contributor

o-alexandrov commented Apr 7, 2025

Related

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

@melloware
Copy link
Collaborator

cc @AllieJonsson @soartec-lab ?

@melloware melloware added the enhancement New feature or request label Apr 7, 2025
@soartec-lab soartec-lab self-assigned this Apr 7, 2025
@AllieJonsson
Copy link
Contributor

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;
}

@o-alexandrov
Copy link
Contributor Author

o-alexandrov commented Apr 7, 2025

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}`>

@AllieJonsson
Copy link
Contributor

Ah i see, so you would like to be able to create a union type instead of enums? That makes sense!

@AllieJonsson
Copy link
Contributor

AllieJonsson commented Apr 7, 2025

@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

@soartec-lab
Copy link
Member

@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?

@AllieJonsson
Copy link
Contributor

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

@soartec-lab
Copy link
Member

@AllieJonsson
However, if you output it using the new method, you won't be able to access it as StatusOrder.NUMBER_24, right?

@AllieJonsson
Copy link
Contributor

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'),
      }
    }
  }
}

@soartec-lab
Copy link
Member

@AllieJonsson

I see, so does this mean that enumGeneration has the following types?

  • const: Current default behavior (useNativeEnums: false)
  • enum: When useNativeEnums: true
  • union: A simple union as proposed in this issue

@AllieJonsson
Copy link
Contributor

@soartec-lab
Yes, that is my proposal!

@soartec-lab
Copy link
Member

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?

@AllieJonsson
Copy link
Contributor

Sure, Ill fix this tomorrow morning!

@soartec-lab
Copy link
Member

You're great!
i change assignment to you.

@AllieJonsson
Copy link
Contributor

Can we change the title of this issue to something along the lines of
Add option to generate union instead of enum? 😊

@o-alexandrov o-alexandrov changed the title Add override.disableEnumGeneration Add option to generate union instead of enum Apr 8, 2025
@melloware melloware added this to the 7.9.0 milestone Apr 9, 2025
@o-alexandrov
Copy link
Contributor Author

o-alexandrov commented Apr 21, 2025

@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:

export const getEnum = (

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

const newEnum = `// eslint-disable-next-line @typescript-eslint/no-redeclare\nexport const ${pascal(

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"
    },
  },
}
}

@AllieJonsson AllieJonsson reopened this Apr 21, 2025
@melloware melloware modified the milestones: 7.9.0, 7.10.0 Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants