From 0067b9d21501158ca86a284ba4df02a44639a66b Mon Sep 17 00:00:00 2001 From: George Fu Date: Thu, 5 Sep 2024 01:04:28 +0000 Subject: [PATCH] fix(types): fix client type transforms --- .changeset/wise-fans-fold.md | 5 +++ packages/types/README.md | 37 +++++++++++++++- packages/types/src/command.ts | 17 +++++++- .../src/transform/client-method-transforms.ts | 11 +++-- .../client-payload-blob-type-narrow.spec.ts | 3 +- .../client-payload-blob-type-narrow.ts | 42 ++++++++++--------- .../types/src/transform/no-undefined.spec.ts | 17 ++++++++ packages/types/src/transform/no-undefined.ts | 23 +++++----- .../codegen/CommandGeneratorTest.java | 2 +- .../codegen/StructureGeneratorTest.java | 2 +- .../codegen/TypeScriptCodegenPluginTest.java | 6 +-- .../endpointsV2/EndpointsV2GeneratorTest.java | 4 +- 12 files changed, 122 insertions(+), 47 deletions(-) create mode 100644 .changeset/wise-fans-fold.md diff --git a/.changeset/wise-fans-fold.md b/.changeset/wise-fans-fold.md new file mode 100644 index 00000000000..3e90737a671 --- /dev/null +++ b/.changeset/wise-fans-fold.md @@ -0,0 +1,5 @@ +--- +"@smithy/types": patch +--- + +fix type transforms diff --git a/packages/types/README.md b/packages/types/README.md index fa78e08552f..7ab3ccd412b 100644 --- a/packages/types/README.md +++ b/packages/types/README.md @@ -34,7 +34,8 @@ const s3b = new S3({}) as UncheckedClient; // and required outputs are not undefined. const get = await s3a.getObject({ Bucket: "", - Key: "", + // @ts-expect-error (undefined not assignable to string) + Key: undefined, }); // UncheckedClient makes output fields non-nullable. @@ -49,6 +50,40 @@ const body = await ( ).Body.transformToString(); ``` +When using the transform on non-aggregated client with the `Command` syntax, +the input cannot be validated because it goes through another class. + +```ts +import { S3Client, ListBucketsCommand, GetObjectCommand, GetObjectCommandInput } from "@aws-sdk/client-s3"; +import type { AssertiveClient, UncheckedClient, NoUndefined } from "@smithy/types"; + +const s3 = new S3Client({}) as UncheckedClient; + +const list = await s3.send( + new ListBucketsCommand({ + // command inputs are not validated by the type transform. + // because this is a separate class. + }) +); + +/** + * Although less ergonomic, you can use the NoUndefined + * transform on the input type. + */ +const getObjectInput: NoUndefined = { + Bucket: "undefined", + // @ts-expect-error (undefined not assignable to string) + Key: undefined, + // optional params can still be undefined. + SSECustomerAlgorithm: undefined, +}; + +const get = s3.send(new GetObjectCommand(getObjectInput)); + +// outputs are still transformed. +await get.Body.TransformToString(); +``` + ### Scenario: Narrowing a smithy-typescript generated client's output payload blob types This is mostly relevant to operations with streaming bodies such as within diff --git a/packages/types/src/command.ts b/packages/types/src/command.ts index 955350fa4b3..101651f1ed7 100644 --- a/packages/types/src/command.ts +++ b/packages/types/src/command.ts @@ -10,7 +10,7 @@ export interface Command< ClientOutput extends MetadataBearer, OutputType extends ClientOutput, ResolvedConfiguration, -> { +> extends CommandIO { readonly input: InputType; readonly middlewareStack: MiddlewareStack; resolveMiddleware( @@ -19,3 +19,18 @@ export interface Command< options: any ): Handler; } + +/** + * @internal + * + * This is a subset of the Command type used only to detect the i/o types. + */ +export interface CommandIO { + readonly input: InputType; + resolveMiddleware(stack: any, configuration: any, options: any): Handler; +} + +/** + * @internal + */ +export type GetOutputType = Command extends CommandIO ? O : never; diff --git a/packages/types/src/transform/client-method-transforms.ts b/packages/types/src/transform/client-method-transforms.ts index 9071032878d..59524b52962 100644 --- a/packages/types/src/transform/client-method-transforms.ts +++ b/packages/types/src/transform/client-method-transforms.ts @@ -1,4 +1,4 @@ -import type { Command } from "../command"; +import type { CommandIO } from "../command"; import type { MetadataBearer } from "../response"; import type { StreamingBlobPayloadOutputTypes } from "../streaming-payload/streaming-blob-payload-output-types"; import type { Transform } from "./type-transform"; @@ -13,23 +13,22 @@ export interface NarrowedInvokeFunction< HttpHandlerOptions, InputTypes extends object, OutputTypes extends MetadataBearer, - ResolvedClientConfiguration, > { ( - command: Command, + command: CommandIO, options?: HttpHandlerOptions ): Promise>; ( - command: Command, + command: CommandIO, cb: (err: unknown, data?: Transform) => void ): void; ( - command: Command, + command: CommandIO, options: HttpHandlerOptions, cb: (err: unknown, data?: Transform) => void ): void; ( - command: Command, + command: CommandIO, options?: HttpHandlerOptions, cb?: (err: unknown, data?: Transform) => void ): Promise> | void; diff --git a/packages/types/src/transform/client-payload-blob-type-narrow.spec.ts b/packages/types/src/transform/client-payload-blob-type-narrow.spec.ts index 19d13652b85..7bd9880846c 100644 --- a/packages/types/src/transform/client-payload-blob-type-narrow.spec.ts +++ b/packages/types/src/transform/client-payload-blob-type-narrow.spec.ts @@ -2,6 +2,7 @@ import type { IncomingMessage } from "node:http"; import type { Client } from "../client"; +import type { CommandIO } from "../command"; import type { HttpHandlerOptions } from "../http"; import type { MetadataBearer } from "../response"; import type { SdkStream } from "../serde"; @@ -87,7 +88,7 @@ interface MyClient extends Client { { interface NodeJsMyClient extends NodeJsClient {} const mockClient = null as unknown as NodeJsMyClient; - const sendCall = () => mockClient.send(null as any, { abortSignal: null as any }); + const sendCall = () => mockClient.send(null as unknown as CommandIO, { abortSignal: null as any }); type A = Awaited>; type B = Omit & { body?: SdkStream }; diff --git a/packages/types/src/transform/client-payload-blob-type-narrow.ts b/packages/types/src/transform/client-payload-blob-type-narrow.ts index f91611c85b2..24c0eb39b1b 100644 --- a/packages/types/src/transform/client-payload-blob-type-narrow.ts +++ b/packages/types/src/transform/client-payload-blob-type-narrow.ts @@ -1,7 +1,8 @@ import type { IncomingMessage } from "http"; import type { ClientHttp2Stream } from "http2"; -import type { InvokeFunction, InvokeMethod } from "../client"; +import type { InvokeMethod } from "../client"; +import type { GetOutputType } from "../command"; import type { HttpHandlerOptions } from "../http"; import type { SdkStream } from "../serde"; import type { @@ -9,7 +10,8 @@ import type { NodeJsRuntimeStreamingBlobPayloadInputTypes, StreamingBlobPayloadInputTypes, } from "../streaming-payload/streaming-blob-payload-input-types"; -import type { NarrowedInvokeFunction, NarrowedInvokeMethod } from "./client-method-transforms"; +import type { StreamingBlobPayloadOutputTypes } from "../streaming-payload/streaming-blob-payload-output-types"; +import type { NarrowedInvokeMethod } from "./client-method-transforms"; import type { Transform } from "./type-transform"; /** @@ -80,12 +82,15 @@ export type BrowserXhrClient = NarrowPayloadBlobTypes */ export type NarrowPayloadBlobOutputType = { [key in keyof ClientType]: [ClientType[key]] extends [ - InvokeFunction, + InvokeMethod, ] - ? NarrowedInvokeFunction - : [ClientType[key]] extends [InvokeMethod] - ? NarrowedInvokeMethod - : ClientType[key]; + ? NarrowedInvokeMethod + : ClientType[key]; +} & { + send( + command: Command, + options?: any + ): Promise, StreamingBlobPayloadOutputTypes | undefined, T>>; }; /** @@ -95,21 +100,18 @@ export type NarrowPayloadBlobOutputType = { */ export type NarrowPayloadBlobTypes = { [key in keyof ClientType]: [ClientType[key]] extends [ - InvokeFunction, + InvokeMethod, ] - ? NarrowedInvokeFunction< + ? NarrowedInvokeMethod< O, HttpHandlerOptions, - Transform, - OutputTypes, - ConfigType + Transform, + FunctionOutputTypes > - : [ClientType[key]] extends [InvokeMethod] - ? NarrowedInvokeMethod< - O, - HttpHandlerOptions, - Transform, - FunctionOutputTypes - > - : ClientType[key]; + : ClientType[key]; +} & { + send( + command: Command, + options?: any + ): Promise, StreamingBlobPayloadOutputTypes | undefined, O>>; }; diff --git a/packages/types/src/transform/no-undefined.spec.ts b/packages/types/src/transform/no-undefined.spec.ts index 97fef69a041..3ee15f4c56d 100644 --- a/packages/types/src/transform/no-undefined.spec.ts +++ b/packages/types/src/transform/no-undefined.spec.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ import type { Client } from "../client"; +import { CommandIO } from "../command"; import type { HttpHandlerOptions } from "../http"; import type { MetadataBearer } from "../response"; import type { Exact } from "./exact"; @@ -114,4 +115,20 @@ type A = { const assert6: Exact = true as const; } } + + { + // Works with outputs of the "send" method. + const c = null as unknown as AssertiveClient; + const list = c.send(null as unknown as CommandIO); + const output = null as unknown as Awaited; + + const assert1: Exact = true as const; + const assert2: Exact = true as const; + const assert3: Exact = true as const; + if (output.r) { + const assert4: Exact = true as const; + const assert5: Exact = true as const; + const assert6: Exact = true as const; + } + } } diff --git a/packages/types/src/transform/no-undefined.ts b/packages/types/src/transform/no-undefined.ts index dab839b01af..2dfb6b495fb 100644 --- a/packages/types/src/transform/no-undefined.ts +++ b/packages/types/src/transform/no-undefined.ts @@ -1,4 +1,5 @@ -import type { InvokeFunction, InvokeMethod, InvokeMethodOptionalArgs } from "../client"; +import type { InvokeMethod, InvokeMethodOptionalArgs } from "../client"; +import type { GetOutputType } from "../command"; /** * @public @@ -62,11 +63,11 @@ type NarrowClientIOTypes = { InvokeMethodOptionalArgs, ] ? InvokeMethodOptionalArgs, NoUndefined> - : [ClientType[key]] extends [InvokeFunction] - ? InvokeFunction, NoUndefined, ConfigType> - : [ClientType[key]] extends [InvokeMethod] - ? InvokeMethod, NoUndefined> - : ClientType[key]; + : [ClientType[key]] extends [InvokeMethod] + ? InvokeMethod, NoUndefined> + : ClientType[key]; +} & { + send(command: Command, options?: any): Promise>>; }; /** @@ -79,9 +80,9 @@ type UncheckedClientOutputTypes = { InvokeMethodOptionalArgs, ] ? InvokeMethodOptionalArgs, RecursiveRequired> - : [ClientType[key]] extends [InvokeFunction] - ? InvokeFunction, RecursiveRequired, ConfigType> - : [ClientType[key]] extends [InvokeMethod] - ? InvokeMethod, RecursiveRequired> - : ClientType[key]; + : [ClientType[key]] extends [InvokeMethod] + ? InvokeMethod, RecursiveRequired> + : ClientType[key]; +} & { + send(command: Command, options?: any): Promise>>>; }; diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/CommandGeneratorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/CommandGeneratorTest.java index 00c2acccb3e..9289684b4a1 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/CommandGeneratorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/CommandGeneratorTest.java @@ -66,7 +66,7 @@ private void testCommmandCodegen(String filename, String[] expectedTypeArray) { .build()) .build(); - new TypeScriptClientCodegenPlugin().execute(context); + new TypeScriptCodegenPlugin().execute(context); String contents = manifest.getFileString(CodegenUtils.SOURCE_FOLDER + "//commands/GetFooCommand.ts").get(); assertThat(contents, containsString("as __MetadataBearer")); diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java index bce45c862ed..4ae57404ef6 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java @@ -489,7 +489,7 @@ private String testStructureCodegenBase( .build()) .build(); - new TypeScriptClientCodegenPlugin().execute(context); + new TypeScriptCodegenPlugin().execute(context); String contents = manifest.getFileString(CodegenUtils.SOURCE_FOLDER + "//models/models_0.ts").get(); if (assertContains) { diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/TypeScriptCodegenPluginTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/TypeScriptCodegenPluginTest.java index 4a59fb6bd26..249ed3db80c 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/TypeScriptCodegenPluginTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/TypeScriptCodegenPluginTest.java @@ -31,7 +31,7 @@ public void generatesRuntimeConfigFiles() { .build()) .build(); - new TypeScriptClientCodegenPlugin().execute(context); + new TypeScriptCodegenPlugin().execute(context); // Did we generate the runtime config files? // note that asserting the contents of runtime config files is handled in its own unit tests. @@ -66,7 +66,7 @@ public void decoratesSymbolProvider() { .build()) .build(); - new TypeScriptClientCodegenPlugin().execute(context); + new TypeScriptCodegenPlugin().execute(context); assertTrue(manifest.hasFile("Foo.ts")); assertThat(manifest.getFileString("Foo.ts").get(), containsString("export class Foo")); @@ -88,7 +88,7 @@ public void generatesServiceClients() { .withMember("packageVersion", Node.from("1.0.0")) .build()) .build(); - new TypeScriptClientCodegenPlugin().execute(context); + new TypeScriptCodegenPlugin().execute(context); assertTrue(manifest.hasFile(CodegenUtils.SOURCE_FOLDER + "/Example.ts")); assertThat(manifest.getFileString(CodegenUtils.SOURCE_FOLDER + "/Example.ts").get(), diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2GeneratorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2GeneratorTest.java index 3ddcf262626..ecc03eeceaa 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2GeneratorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2GeneratorTest.java @@ -10,7 +10,7 @@ import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.typescript.codegen.CodegenUtils; -import software.amazon.smithy.typescript.codegen.TypeScriptClientCodegenPlugin; +import software.amazon.smithy.typescript.codegen.TypeScriptCodegenPlugin; public class EndpointsV2GeneratorTest { @Test @@ -79,7 +79,7 @@ private MockManifest testEndpoints(String filename) { .build()) .build(); - new TypeScriptClientCodegenPlugin().execute(context); + new TypeScriptCodegenPlugin().execute(context); assertThat(manifest.hasFile(CodegenUtils.SOURCE_FOLDER + "/endpoint/EndpointParameters.ts"), is(true));