Hi @tvald,

We do use string literals in the TypeScript declaration files, but only when a shape is explicitly modeled as an enumeration in the model files shared by the AWS SDKs. For example, EC2's InstanceType shape is modeled as an enum
and is therefore defined as a union of string literals in the corresponding .d.ts file. I'll see if the service is open to changing this value in their model from a string to an enum.

I would like to see also union for region.

// instead of
type AwsRegion = string;
// we could use
type AwsRegion = 'us-east-1' | 'eu-central-1' | 'eu-west-1' | ...;

This could prevent errors like

new AWS.Lambda({ region: 'us-eest-1' });

It seems the ts-generator appends a | string at the end.

For example, this converts S3.BucketCannedACL to this

I am trying to generate schemas automatically from the type-definitions, and this appendage is causing significant grief, requiring post processing.

The underlying API call will not honor it, so not sure there is a legitimate purpose for it. Or am I missing something?

Happy to open a new issue and submit a PR if that is the best course of action here.

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.

This seems like a simple enough fix and is an obvious bug IMHO:
An enum is by definition limited to the set of valid values.
The current implementation allows the typescript type to exceed that limit.

Will a PR be accepted for this?

Will a PR be accepted for this?

Would love to accept a PR

@sramam

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.

oh you github action!!!
It took 15 months to get a response and you give me a week?!
I'll be taking a stab at this - it won't be 15 months, but it's not likely to be next week either...

@sramam, apologies there, my comment was added while I went through all the feature requests, since the response requested label was applied (which was just for the sake that a response was requested from you) actions did its job because it is set to 7 days, I agree with you with you about delayed response on this but didn't mean to close it.

Not a problem - I wasn't complaining, just trying to be funny while staying pertinent.
Good thing I didn't chose a career in comedy no?!

Would love to accept a PR
I can raise a PR with a little guidance. I have been working with the SDK of late and I noticed the issue.

@SarthakC, that'd be really awesome

I believe all that is needed is to convert }).join('|') + '|' + stringType; => }).join('|'); on this line.

This change is simple, but might result in the failure of some tests. I'm willing to help if you get stuck.

Maybe it could be better to just use a LiteralUnion: microsoft/TypeScript#29729 (comment)

Interesting idea @Shinigami92.
What benefits do you see from a move to a LiteraUnion?

You can still pass any string to a LiteralUnion but get autocompletion for known literals
So if e.g. a new region is added you can pass it and you are not forced to ignore ts with a comment annotation

Hope we can add mars-west-1 soon :P

Unless I am misunderstanding something, isn't that the current implementation?
The issue as I understand it was meant to disallow invalid/unsupported values.
That's how enums work in all languages that I am aware of.

When mars-west-1 is supported by AWS, the enum will be updated and a new version published.
Isn't it reasonable to expect that one would have to upgrade aws-sdk version to use the new region?

No, the current enum/literal don't provide autocompletion in your IDE!

Also e.g. in tests you can find things like this:

@Shinigami92 I might mistaken in the previous comment.

// The current implementation renders this:
type Region = 'us-east-1' | 'us-west-1' | string;

const region: Region = 'us-east-2'


// The requested implementation should render this:
type RegionStrict = 'us-east-1' | 'us-west-1';

const regionStrict: RegionStrict = 'us-east-2'

If you put this snippet in the typescript playground, you'll see that option 2 is the right one - even per your comments from earlier in the thread.

The base aws spec is an enum, but the typescript code generates a Union of StringLiterals.
Which I agree is the most ergonomic.

I'd still vote for: }).join('|') + '|' + stringType; => }).join('|');

Mh yeah, if you name it RegionStrict you are right
But I was hoping to type e.g. configs like new AWS.S3({ region: _ }) were _ gets nice autocompletion

IDE auto-complete

The lack of auto-complete in that case seems to be stemming from this, not the way the type is generated in the ts-generator.

When defined properly, it'll work fine I think.

I am not seeing the difference between the LiteralUnion as you referenced and TS default behavior when it's just a union of string literals.

Test case

The test case you pointed out is however an issue I imagine.
I don't understand how and where regions like xx-west-1 are used - is it only for tests or in the SDK proper?
If the core team could provider pointers, it'd be appreciated.

LiteralUnion<... | string> doesn't make sense at all ⚠️
Try your first and second example screenshot and try autocompletion AND a custom region
The LiteralUnion<'eu-west-1'> will provide a suggestion but also allow custom / unknown regions

What is a "custom region"? When are these valid?

Looking at the code, the underlying schema specifies the type as an enum which is a non-extensible type in my understanding.
You are asking for all enum definitions across aws-sdk to be extensible - which sounds really wrong, even dangerous to me.

For example here, how is us-weest-1 different from mars-west-1? While auto-complete works for your narrow case,
the type-checking is broken.

I must be missing something really fundamental.

Could you provide concrete examples of when this would be useful?

[edit: fix markdown formatting]

[edit2:]
oh - and even that auto-complete improvement would only work with the stricter type definition.
Which is something this change would not address.

I personally don’t know any but for tests like the one above
One benefit would be to not break anyones code/be backward compatible (at least for a temporary amout of time)
Maybe you forgot a region or another case 🤷
Yeah... you can still force with // ts-expect-error

We could also build a RegionStrict = 'eu-west-1' | ... together with Region = LiteralUnion<RegionStrict>
Then use Region in S3 config

Either it's strict or it's not. The specification being used by this generator is strict.
There should be no valid code that works with an underlying strict expectation and a looser definition in the client code.
If such code exists, all this will do is help convert a run time error into a compile time error.

I'd still vote for: }).join('|') + '|' + stringType; => }).join('|'); and fix any issues that crop up - in tests or otherwise.

--

The test case you pointed out seems to only instantiate an object for the signature without actually making any calls to AWS.
More-over, the current definition of AWS.S3.ClientConfiguration.region is a string. So even the typescript compiler
doesn't complain.

I did not find any other instances with simple grep style searching - but the CI will report any misses.

--

@SarthakC, hopefully this long discussion hasn't intimidated you.
IMHO, the right change is still the simple removal of the | string. from enum definitions.
Please do try and create the pull request. I'd be happy to assist where I can.

Eventually, I am also just a user of the library - the core team will make the decision to accept it or not.

Any progress at all on this? It is really annoying not having the proper ENUM functionality on a typescript system for a strictly defined model

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.