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

Support for exactOptionalPropertyTypes #4017

Open
1 of 2 tasks
laverdet opened this issue Oct 4, 2022 · 4 comments
Open
1 of 2 tasks

Support for exactOptionalPropertyTypes #4017

laverdet opened this issue Oct 4, 2022 · 4 comments
Assignees
Labels
feature-request New feature or enhancement. May require GitHub community feedback. queued This issues is on the AWS team's backlog

Comments

@laverdet
Copy link

laverdet commented Oct 4, 2022

Describe the feature

TypeScript 4.4 added support for a setting in tsconfig.json: exactOptionalPropertyTypes. See the announcement here:
https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#exact-optional-property-types

With this flag enabled, undefined is no longer assignable to optional members unless they explicitly have undefined in their signature.

The AWS SDK gracefully handles the case where a parameter is specified but undefined. Specifically, the overwhelming majority of the SDK transmits requests with JSON.stringify which will omit undefined values from the payload entirely. Therefore, the AWS SDK should add undefined to most of the generated d.ts input types.

Use Case

exactOptionalPropertyTypes brings the types of a project closer to the actual shape during runtime. It should be used in all new projects when possible. Currently, using the AWS SDK with this option enabled is unreasonably cumbersome.

Proposed Solution

I believe this can be implemented with minor changes to the code generation. Instead of outputting ?: you will output ?: undefined | .

Other Information

This is the PR of the feature from TypeScript microsoft/TypeScript#43947

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

SDK version used

v3.184.0

Environment details (OS name and version, etc.)

N/A

@laverdet laverdet added feature-request New feature or enhancement. May require GitHub community feedback. needs-triage This issue or PR still needs to be triaged. labels Oct 4, 2022
@yenfryherrerafeliz yenfryherrerafeliz self-assigned this Mar 13, 2023
@yenfryherrerafeliz yenfryherrerafeliz added p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Mar 13, 2023
@yenfryherrerafeliz yenfryherrerafeliz added the needs-review This issue/pr needs review from an internal developer. label Nov 7, 2023
@aBurmeseDev aBurmeseDev added investigating Issue is being investigated and/or work is in progress to resolve the issue. and removed needs-review This issue/pr needs review from an internal developer. labels Nov 22, 2023
@aBurmeseDev aBurmeseDev added needs-triage This issue or PR still needs to be triaged. and removed investigating Issue is being investigated and/or work is in progress to resolve the issue. labels Apr 3, 2024
@aBurmeseDev aBurmeseDev removed needs-triage This issue or PR still needs to be triaged. p2 This is a standard priority issue labels Apr 22, 2024
@aBurmeseDev
Copy link
Member

Hi @laverdet - sorry for the long wait.

We recently had a team discussion about this and my colleague left a comment with the explanation and workaround on the other similar issue: #5992

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.

We would still be happy to further assist you if you have any specific issue around this. Thanks again for reaching out.

@aBurmeseDev aBurmeseDev closed this as not planned Won't fix, can't repro, duplicate, stale Apr 22, 2024
@laverdet
Copy link
Author

@aBurmeseDev I fear that we have had a miscommunication. I will take blame for that because I did not provide enough information.

The ? member declaration and undefined types are not mutually exclusive. Generally, if ? is specified then | undefined should also be specified. The exceptions where ? should appear without undefined are vanishingly small.

As a case study, the DefinitelyTyped project on GitHub (which represents the entire @types/* namespace on npm) automated this process in their repository over 45 commits. Here is one of those commits: DefinitelyTyped/DefinitelyTyped@dd29479

This series of commits touches over 500,000 lines of declarations and is nothing but this kind of thing:

--- a/types/zarinpal-checkout/index.d.ts
+++ b/types/zarinpal-checkout/index.d.ts
@@ -14,8 +14,8 @@ declare namespace ZarinPal {
         Amount: number;
         CallbackURL: string;
         Description: string;
-        Email?: string;
-        Mobile?: string;
+        Email?: string | undefined;
+        Mobile?: string | undefined;
     }
 
     interface PaymentRequestOutput {

The issue is not that AWS should remove | undefined from the output types, but that AWS should add it to the input types.

For a concrete example, as it relates to AWS, consider:

import type { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { GetItemCommand } from "@aws-sdk/client-dynamodb";

declare const client: DynamoDBClient;

// Compiles fine.
await client.send(new GetItemCommand({
	Key: {},
	TableName: "",
}));

const ConsistentRead = undefined;

// Argument of type '{ Key: {}; TableName: string; ConsistentRead: undefined; }' is not assignable
// to parameter of type 'GetItemCommandInput' with 'exactOptionalPropertyTypes: true'. Consider
// adding 'undefined' to the types of the target's properties.
//   Types of property 'ConsistentRead' are incompatible.
//   Type 'undefined' is not assignable to type 'boolean'
await client.send(new GetItemCommand({
	Key: {},
	TableName: "",
	ConsistentRead,
}));

The second invocation fails since ConsistentRead is specified but undefined. Casting to AssertiveClient or UncheckedClient, as recommended in the documentation you linked, will not change the result.

This is the tsconfig.json:

{
	"compilerOptions": {
		"module": "esnext",
		"moduleResolution": "nodenext",
		"target": "esnext",
		"strict": true,
		// 🚨 This was added in TypeScript 4.4 https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#exact-optional-property-types
		"exactOptionalPropertyTypes": true,
	},
}

@laverdet
Copy link
Author

@aBurmeseDev would you mind reopening this issue? If I don't hear back I'll open a new one because I understand the team gets a lot of notifications, but there has been a misunderstanding here.

@aBurmeseDev aBurmeseDev reopened this Apr 25, 2024
@aBurmeseDev
Copy link
Member

@laverdet - thanks for providing detailed information, I'll look more into it and discuss it with the team.

@aBurmeseDev aBurmeseDev added the investigating Issue is being investigated and/or work is in progress to resolve the issue. label Apr 25, 2024
@kuhe kuhe self-assigned this May 7, 2024
@kuhe kuhe added queued This issues is on the AWS team's backlog and removed investigating Issue is being investigated and/or work is in progress to resolve the issue. labels May 7, 2024
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. queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

4 participants