orval icon indicating copy to clipboard operation
orval copied to clipboard

Add option to generate union instead of enum

Open o-alexandrov opened this issue 7 months ago • 16 comments

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

o-alexandrov avatar Apr 07 '25 05:04 o-alexandrov

cc @AllieJonsson @soartec-lab ?

melloware avatar Apr 07 '25 11:04 melloware

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

AllieJonsson avatar Apr 07 '25 12:04 AllieJonsson

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

o-alexandrov avatar Apr 07 '25 13:04 o-alexandrov

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

AllieJonsson avatar Apr 07 '25 15:04 AllieJonsson

@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

AllieJonsson avatar Apr 07 '25 18:04 AllieJonsson

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

soartec-lab avatar Apr 08 '25 10:04 soartec-lab

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 avatar Apr 08 '25 10:04 AllieJonsson

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

soartec-lab avatar Apr 08 '25 12:04 soartec-lab

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 avatar Apr 08 '25 13:04 AllieJonsson

@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

soartec-lab avatar Apr 08 '25 13:04 soartec-lab

@soartec-lab Yes, that is my proposal!

AllieJonsson avatar Apr 08 '25 13:04 AllieJonsson

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?

soartec-lab avatar Apr 08 '25 13:04 soartec-lab

Sure, Ill fix this tomorrow morning!

AllieJonsson avatar Apr 08 '25 13:04 AllieJonsson

You're great! i change assignment to you.

soartec-lab avatar Apr 08 '25 13:04 soartec-lab

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

AllieJonsson avatar Apr 08 '25 17:04 AllieJonsson

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

o-alexandrov avatar Apr 21 '25 11:04 o-alexandrov

bump

LeOndaz avatar Sep 03 '25 20:09 LeOndaz