cdk8s-team / cdk8s-cli

Apache License 2.0
38 stars 23 forks source link

[cdk8s-cli] Error when importing enum values that start with a numeric character #40

Closed eladb closed 2 years ago

eladb commented 4 years ago

Description of the bug:

For example, this is from postgressqlinstance.database.crossplane.io:

/**
 * EngineVersion specifies the desired PostgreSQL engine version. Allowed Versions: 9.6 and 11.
 *
 * @schema PostgreSqlInstanceSpecEngineVersion
 */
export enum PostgreSqlInstanceSpecEngineVersion {
  /** 9.6 */
  9_6 = "9.6",
  /** 11 */
  11 = "11",
}

Oouch....


This is :bug: Bug Report

jvassev commented 3 years ago

As far as Typescript is concerned, isn't it better to just generate a type union?


engine: "9.5" | "11";

Another deficiency of enums (at least in Typescript) is the super long names that are auto-geneareted. For example

import {
  VerticalPodAutoscaler,
  VerticalPodAutoscalerSpecResourcePolicyContainerPoliciesControlledResources as res,
} from '../imports/autoscaling.k8s.io';

I have to rename the long type name to res so I can use res.CPU or res.MEMORY.

What do you think about changing the emitted code to use string litterals?

jvassev commented 3 years ago

Now that I think of it, we can have it both. For the example of autoscaling:



/**
 * @schema VerticalPodAutoscalerSpecResourcePolicyContainerPoliciesControlledResources
 */
export enum VerticalPodAutoscalerSpecResourcePolicyContainerPoliciesControlledResources {
  /** cpu */
  CPU = "cpu",
  /** memory */
  MEMORY = "memory",
}

export interface VerticalPodAutoscalerSpecResourcePolicyContainerPolicies {

  /**
   * @schema VerticalPodAutoscalerSpecResourcePolicyContainerPolicies#controlledResources
   */
  readonly controlledResources?: (VerticalPodAutoscalerSpecResourcePolicyContainerPoliciesControlledResources | "cpu" | "memory")[];

}
iliapolo commented 3 years ago

@jvassev We try and avoid using type unions because they don't translate well to other languages. Since the type script generation is the base for other languages bindings, it would be tricky to special case it.

See https://aws.github.io/jsii/specification/2-type-system/#type-unions

Perhaps this can shift to a jsii feature request. But I can see this getting rather complex.

github-actions[bot] commented 3 years ago

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

github-actions[bot] commented 3 years ago

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

github-actions[bot] commented 3 years ago

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

github-actions[bot] commented 2 years ago

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

github-actions[bot] commented 2 years ago

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.