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

VSCode allowedValues Output string literal Union type - Asking for enum #642

Open
damntrecky opened this issue Dec 8, 2023 · 3 comments

Comments

@damntrecky
Copy link
Contributor

damntrecky commented Dec 8, 2023

Is your feature request related to a problem? Please describe.
The current VSCode allowedValues outputs a string literal union type in Typescript:

export type LambdaRuntime = 'dotnetcore2.1' | 'nodejs12.x'

The other implementations for Kotlin and C# output an enum type. The Typescript code standard in PR's keep asking for Enums as the preferred implementation, so we are converting the String literal Union types output from this package to hardcoded Enums... This process seems backwards.

Describe the solution you'd like
I would like the Typescript allowedValues to ouput an Enum type

export  enum LambdaRuntime{
    Dotnetcore21 = 'dotnetcore2.1',
    Nodejs12x = 'nodejs12.x'
}

Describe alternatives you've considered
I have not

Additional context

Kotlin output for enum

/**
 * The lambda runtime
 */
public enum class LambdaRuntime(
    private val `value`: String,
) {
    Dotnetcore21("dotnetcore2.1"),
    Nodejs12x("nodejs12.x"),
    Unknown("unknown"),
    ;

    public override fun toString(): String = value

    public companion object {
        public fun from(type: String): LambdaRuntime = values().firstOrNull { it.value == type }
                ?: Unknown
    }
}

All code snippets referenced from test files generatorOutput.ts and testGeneratorOutput for Kotlin and Typescript within this package.

@damntrecky
Copy link
Contributor Author

If this works and is accepted, it would be a breaking change and not backwards compatible. So for that reason, it may not be possible to make this update.

@damntrecky
Copy link
Contributor Author

The Typescript string literal type definition says:

In practice string literal types combine nicely with union types, type guards, and type aliases. You can use these features together to get enum-like behavior with strings.

Which I think works fine. We keep getting comments on poor implementation for allowedValues and should use Enum types as its the "preferred standard" for reviewers.

I would expect consistency in the implementation here across languages, if possible.

@justinmk3
Copy link
Contributor

The Typescript code standard in PR's keep asking for Enums as the preferred implementation

What standard is requesting that? Enums in typescript are rarely, if ever, needed. Union types can be made "iterable", see https://stackoverflow.com/a/60041791/152142

Typescript string literal type definition says:

In practice string literal types combine nicely with union types, type guards, and type aliases. You can use these features together to get enum-like behavior with strings.

Which I think works fine. We keep getting comments on poor implementation for allowedValues

We can enhance telemetry generator to generate iterable names for the union types.

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

No branches or pull requests

2 participants