Skip to content
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

Generate enum "StringLiterals" as name implies #1165

Closed
wants to merge 1 commit into from
Closed

Generate enum "StringLiterals" as name implies #1165

wants to merge 1 commit into from

Conversation

onionhammer
Copy link

@onionhammer onionhammer commented Apr 25, 2020

For the given enum

    public enum AccountType
    {
        User,
        Moderator,
        Administrator
    }

Rather than generating

export type AccountType = 0 | 1 | 2;

Which is not very useful, generate

export type AccountType =
      "User" 
    | "Moderator" 
    | "Administrator";

Note this would be a breaking change to anyone who regenerates their model, but I can't actually see the use for this feature as it exists today.

Rather than generating

```ts
export type AccountType = 0 | 1 | 2;
```

Which is not very useful, generate 

```ts
export type AccountType =
      "User" 
    | "Moderator" 
    | "Administrator";
```
@RicoSuter
Copy link
Owner

Thanks for the PR. But I think it is just wrong: The default serialization behavior of JSON.NET is to serialize enums as integer values (instead of string) - this is why we generate 0 | 1 | 2, so that it matches the serializer behavior - you cannot change that in the code generator. If you want strings then you need to add [JsonConverter(typeof(StringEnumConverter))] on the property, enum or globally and then it would also generate the code you expect.

@onionhammer
Copy link
Author

onionhammer commented Apr 27, 2020

@RicoSuter In what way is literal 1, 2, or 3 a string? Ints are already convertible to enums without any special converter. And in what way is 1 , 2, or 3 going to tell you something meaningful about an AccountType?

Additionally, if, as you propose, you use StringEnumConverter, the value you will receive from JSON.NET serialized data for AccountType.User is "User", not 0.. additionally ASPNET will handle "User" as an incoming value and deserialize it just as easily as 0.

Per the docs:
"Converts an Enum to and from its name string value."

@RicoSuter
Copy link
Owner

If you use StringEnumConverter then the value (and name) are both strings and the AccountType should be with strings, is this not the case?

@onionhammer
Copy link
Author

onionhammer commented May 8, 2020

@RicoSuter using StringEnumConverter has nothing to do with how the typescript is generated, only with how JSON is serialized from your ASP.NET application. This issue has to do with how the typescript is generated.

Currently the way NJsonSchema generates typescript with the setting called "StringLiterals" for enum style has nothing to do with strings. It generates numbers (0 | 1 | 2).

If you use StringEnumConverter in your ASP.NET application, it will return information like this:

{ "SomeEnum": "SomeValue" }

In tandem with "StringLiterals" setting for enum style, the typescript will be:

interface Model {
  "SomeEnum": 0 | 1 | 2
}

Which is not useful

@RicoSuter
Copy link
Owner

RicoSuter commented May 8, 2020

If you dont set StringEnumConverter then the serializer expects ints and you cannot just generate strings because it would not match the serializer settings (ie the client does not work)

@onionhammer
Copy link
Author

onionhammer commented May 8, 2020

@RicoSuter The json deserializer in asp.net will work regardless of whether you set StringEnumConverter, it's smart enough to handle int or string.

@RicoSuter
Copy link
Owner

RicoSuter commented May 8, 2020

The json deserializer in asp.net will work regardless of whether you set StringEnumConverter, it's smart enough to handle int or string.

Ok that might be true for asp.net but if the spec specifies the values as 1,2,3 then only these values are allowed and we cannot just generate strings.. you cannot assume that it works with the names (x-enum-names) with any backend.

... the solution is to just provide a correct spec, in your case a spec with string values.

@onionhammer
Copy link
Author

@RicoSuter Can you give me an example use-case of when this would come in handy

interface Model {
  SomeEnum: 0 | 1 | 2
}

What does 0 mean? What does 1 mean? What does 2 mean? From a typescript POV they are just meaningless numbers.

Additionally, can you please explain what 0 | 1 | 2 has to do with "StringLiterals" ?

@onionhammer
Copy link
Author

onionhammer commented May 13, 2020

@RicoSuter Looks like you fixed this in a separate commit, not sure what the disagreement was about though; the latest version of NJsonSchema now generates string literals properly.

Fixed in #1014 (WIP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants