aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.04k stars 569 forks source link

Support for `exactOptionalPropertyTypes` #4017

Open laverdet opened 1 year ago

laverdet commented 1 year ago

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 https://github.com/microsoft/TypeScript/pull/43947

Acknowledgements

SDK version used

v3.184.0

Environment details (OS name and version, etc.)

N/A

aBurmeseDev commented 4 months ago

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: https://github.com/aws/aws-sdk-js-v3/issues/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.

laverdet commented 4 months ago

@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: https://github.com/DefinitelyTyped/DefinitelyTyped/commit/dd29479f653d177022778c549e9fde907575fa06

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 commented 4 months ago

@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 commented 4 months ago

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

laverdet commented 3 months ago

@aBurmeseDev @kume @RanVaknin please let me know if you need any more information on this.

aBurmeseDev commented 3 months ago

@laverdet - will do and feel free to check back in for further updates.