Typescript union types will not be compatible with other languages.
This will need to be represented by a single type. The way we do it in the cdk is to create a union like class with APIs like fromInstanceType and fromString that return that type.

I see, I can define a instanceType class here to use it as the type of this field, something like this:

export class InstanceType {
  public static fromEc2InstanceType(instanceType: ec2.InstanceType) {
    return new InstanceType(`ml.${instanceType}`);
  }

  public static fromString(instanceType: string) {
    return new InstanceType(instanceType);
  }

  constructor(private readonly instanceType: string) {
  }

  public toString(): string {
    return this.instanceType;
  }
}

But it won't be backward compatible, e.g. current users need to change their code: instanceType: ec2.InstanceType.of(ec2.InstanceClass.P3, ec2.InstanceSize.XLARGE2). Should we make it backward compatible somehow, or it's fine to do this?

Hi @shivlaks, could you take a quick look at the above comments when you get a chance? Thanks!

Hi @shivlaks, looks like yarn compat is not happy with this change, because it changed the API. So should I make the InstanceType class defined in above to extend ec2. InstanceType?

I know we removed the @experimental annotations after you had initially published this PR, but I believe this is now a breaking change.

Let me check with a team member or two to see whether we would need to:

1. support the old parameter but mark it as @deprecated
2. make this change as it is, but call it out as a BREAKING change.

It is a breaking change if we ship it as it is as clients who upgrade will see breakage.

Right, in earlier comments I was checking whether we should make it backward compatible. Let me know if we have to extend ec2. InstanceType here, thanks

Hi @shivlaks, any updates on this?

As Shiv mentions above, we will need to create a new property (maybe mlInstanceType) and mark the old property as deprecated. Unfortunately we cannot extend this type with a type union due to jsii restrictions