openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG] [Typescript] Impossible to use enum options

Open ksvirkou-hubspot opened this issue 2 years ago • 10 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [x] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [x] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description
export declare type FilterOperatorEnum = "EQ" | "NEQ" | "LT" | "GT";

I can't use any option of the enum

let operation = FilterOperatorEnum.EQ
openapi-generator version

6.2.1

OpenAPI declaration file content or url
"Filter": {
    "type": "object",
    "properties": {
        "operator": {
            "type": "string",
            "enum": [
               "EQ",
               "NEQ",
               "LT",
               "GT",
            ]
        }
    }
}
Generation Details

generate -i *.json -g typescript -o /dir --skip-validate-spec --additional-properties usePromises=true,supportsES6=true,platform=node

Related issues/PRs

https://github.com/OpenAPITools/openapi-generator/issues/13509

Suggest a fix

ksvirkou-hubspot avatar Jan 31 '23 10:01 ksvirkou-hubspot

Can we please label this issue as breaking change without fallback in v7, @wing328 ? Strings and enums are completely different things in typescript. (Cf. https://github.com/OpenAPITools/openapi-generator/pull/14663/files#diff-669466c5a2b36b6286325ddca08226df4ed6f90dd04f101ff840bae691a984d1L23)

I don't really see the reason for this change though. If you want to use an option, you can just use the string, e.g. "EQ". Typescript will correctly make sure that only allowed options are used and assigning dynamic strings to such literal string unions is also prevented. Also literal string unions are more ideomatic in typescript's structural typing.

One advantage of enums is the possibility of runtime checking.

When enums with the same values are defined in multiple APIs, string literals can be used in both places, while the material enums cannot. Whether that is a good thing or not depends on the specific use case. E.g. in our situation we have shared domain types which are imported in mutliple API definitions, but for code generation purposes the imported types will be duplicated.

We will override the template here, but maybe this should be togglable with a generator argument?

bodograumann avatar Sep 07 '23 09:09 bodograumann

This seems to be a step backwards. As @bodograumann commented, string union properties are much more idiomatic and nicer to work with in Typescript than string enums.

As you can see in this section of the diff from the PR: https://github.com/OpenAPITools/openapi-generator/pull/14663/files#diff-093571dbeb012904d7da944caca6939594eae68044e29bb75b0584146fd02cda

        pet.status = 'available'

now needs to be written as:

        pet.status = petstore.PetStatusEnum.Available

which makes client code much harder to write and work with.

This should definitely be an option, and probably off by default :-(

RichardBradley avatar Mar 12 '24 21:03 RichardBradley

I came here looking to maybe submit a PR to change enum handling from string enums to string unions in the "typescript-rxjs" generator, because it would make our client much nicer and perhaps many others. I guess I won't attempt that, if you're going in the other direction on this generator.

Why are there 11 different "typescript*" generators, and how am I meant to pick between them? They appear to handle union types differently and handle enums differently.

RichardBradley avatar Mar 12 '24 22:03 RichardBradley

The "typescript" generator without any suffix is a rewrite and was supposed to replace all the others, but it seems there is not much pressure to actually do that now (@TiFu ?). So I would suggest using that one for any new projects.

Regarding the enums I didn't get any feedback on my comment above from the maintainers, but it shouldn't be too hard to introduce a attribute to toggle between literal strings and enum objects.

bodograumann avatar Mar 13 '24 06:03 bodograumann

Thanks for your reply

The "typescript" generator without any suffix is a rewrite and was supposed to replace all the others, but it seems there is not much pressure to actually do that now (@TiFu ?).

An abandoned attempt at https://xkcd.com/927/ ? Classic.

I can't see any documentation about what each of the 11 is for or why I might choose one over the other. All 11 are listed here, but there's no summary of them, nor do each give an intro about what they are for and how they differ from the other 10.

If the "typescript (experimental)" generator is intended as a replacement for all the others, I might have expected it to have a summary of that goal in its docs, along with some discussion of the problems it intends to solve and why the other 10 are unsuitable.

As far as I can tell, the primary difference seems intended to be which HTTP client library is bundled by default.

However, they do also differ significantly in how they handle "AllOf" combined classes and enums, so I find this situation very frustrating.

RichardBradley avatar Mar 13 '24 13:03 RichardBradley

Yeah that reference is kinda apt. I think the goal was to only have to support a single template and get rid of all the code duplication. Unfortunately once the generator works well enough, there is little motivation to contribute anything in this direction or improve the documentation...

If you like I can work with you to bring the literal string enums back. Alternatively I can give you the template override that we are using to get it locally.

bodograumann avatar Mar 13 '24 18:03 bodograumann

@bodograumann is correct. I had started this a few years back - with the goal of replacing all the code duplication. There even is an issue that documents this: #802 including motivation and the rough architectural plan.

With personal stuff taking over more and more of my time (such as a full-time job ;) truly missing the life of a college student haha), progress on this also stopped. Of course it's open source so if anyone feels like adding to this, they can always pick up where I have left off.

TiFu avatar Mar 13 '24 18:03 TiFu

@bodograumann I am sorry for silence. Can I add an option (a generator argument) for enum's type? example --enumType=stringUnions( or literalStrings) --enumType=enum (enumObjects) Default will be stringUnions.

ksvirkou-hubspot avatar Mar 15 '24 09:03 ksvirkou-hubspot

That would be greatly appreciated, @ksvirkou-hubspot ! I like the proposed argument scheme and would go for stringUnion vs enum as options. Please let me know if you need any help.

bodograumann avatar Mar 15 '24 10:03 bodograumann

https://github.com/OpenAPITools/openapi-generator/pull/18531 Could you check it, please?

ksvirkou-hubspot avatar Apr 29 '24 09:04 ksvirkou-hubspot

Still waiting on the next release. But I think it should be fine :-) Thanks for adding this back, @ksvirkou-hubspot

bodograumann avatar Jun 20 '24 14:06 bodograumann