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

Typescript types for required and optional attributes are not generated correctly #5992

Open
2 of 3 tasks
rehos opened this issue Apr 11, 2024 · 3 comments
Open
2 of 3 tasks
Assignees
Labels
feature-request New feature or enhancement. May require GitHub community feedback. p2 This is a standard priority issue queued This issues is on the AWS team's backlog

Comments

@rehos
Copy link

rehos commented Apr 11, 2024

Checkboxes for prior research

Describe the bug

Typescript types are for required and optional attributes are not generated corrrectly. I.e. required attributes can accept undefined. And optional attributes cannot accept undefined when setting the typescript compilation option exactOptionalPropertyTypes to true.

Fixing this will probably also fix this issue

SDK version number

3.540.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.11.0

Reproduction Steps

Just look at the generated typescript types for all clients

Observed Behavior

Typescript types of required attributes are all generated as a union with their type and undefined. And optional attributes are not generated as a union with undefined which gives problems when setting the typescript compilation option exactOptionalPropertyTypes to true.

See for example the in the DynamoDB client the TableName and IndexName attributes in the type QueryInput:

export interface QueryInput {
  TableName: string | undefined;
  IndexName?: string;

 // omitted other attributes

Expected Behavior

I expect required attributes cannot be set to undefined while optional attributes can accept undefined. Also when setting the typescript compilation option exactOptionalPropertyTypes to true.

So I expected that the generated code for the example above is generated as:

export interface QueryInput {
  TableName: string;
  IndexName?: string | undefined;

  // omitted other attributes

Possible Solution

Probably the following line in DocumentClientCommandGenerator.java:

String typeSuffix = isRequiredMember(member) ? " | undefined" : "";

should be changed to:

String typeSuffix = isRequiredMember(member) ? "" : " | undefined";

Additional Information/Context

No response

@rehos rehos added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 11, 2024
@rehos rehos changed the title Typescript types are for required and optional attributes are not generated correctly Typescript types for required and optional attributes are not generated correctly Apr 11, 2024
@kuhe kuhe added feature-request New feature or enhancement. May require GitHub community feedback. and removed bug This issue is a bug. labels Apr 12, 2024
@RanVaknin
Copy link
Contributor

Hi @rehos ,

Thanks for reaching out. The reason why we generate " | undefined" for our types, is intentionally designed for forward compatibility reasons. This allows AWS services to change their APIs by making previously required parameters optional, ensuring that existing client applications will not break when the API becomes less strict.

To improve the typescript experience, you can cast your client to be an UncheckedClient or AssertiveClient from Smithy-Typescript package. Just be mindful that using UncheckedClient eliminates compiler-generated nullability warnings for output fields by treating them as non-nullable. You should still perform manual type checks as necessary for your application's specific needs.

Thanks,
Ran~

@RanVaknin RanVaknin self-assigned this Apr 12, 2024
@RanVaknin RanVaknin added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Apr 12, 2024
@rehos
Copy link
Author

rehos commented Apr 13, 2024

@RanVaknin, thank you for the clarification for required types. For optional types I think the generated code should also generate the " | undefined" to avoid compiler errors when setting the typescript compilation option exactOptionalPropertyTypes to true. We set this to true for our code base to get the most benefits when using @effect/schema.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Apr 14, 2024
@kuhe
Copy link
Contributor

kuhe commented Apr 15, 2024

TS playground demonstrating the secondary issue: TSP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or enhancement. May require GitHub community feedback. p2 This is a standard priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

3 participants