Skip to content

Commit

Permalink
fix(types): fix client type transforms
Browse files Browse the repository at this point in the history
  • Loading branch information
kuhe committed Sep 5, 2024
1 parent 12d9afe commit 0067b9d
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 47 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-fans-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@smithy/types": patch
---

fix type transforms
37 changes: 36 additions & 1 deletion packages/types/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const s3b = new S3({}) as UncheckedClient<S3>;
// 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.
Expand All @@ -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<S3Client>;

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<T>
* transform on the input type.
*/
const getObjectInput: NoUndefined<GetObjectCommandInput> = {
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
Expand Down
17 changes: 16 additions & 1 deletion packages/types/src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface Command<
ClientOutput extends MetadataBearer,
OutputType extends ClientOutput,
ResolvedConfiguration,
> {
> extends CommandIO<InputType, OutputType> {
readonly input: InputType;
readonly middlewareStack: MiddlewareStack<InputType, OutputType>;
resolveMiddleware(
Expand All @@ -19,3 +19,18 @@ export interface Command<
options: any
): Handler<InputType, OutputType>;
}

/**
* @internal
*
* This is a subset of the Command type used only to detect the i/o types.
*/
export interface CommandIO<InputType extends object, OutputType extends MetadataBearer> {
readonly input: InputType;
resolveMiddleware(stack: any, configuration: any, options: any): Handler<InputType, OutputType>;
}

/**
* @internal
*/
export type GetOutputType<Command> = Command extends CommandIO<any, infer O> ? O : never;
11 changes: 5 additions & 6 deletions packages/types/src/transform/client-method-transforms.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -13,23 +13,22 @@ export interface NarrowedInvokeFunction<
HttpHandlerOptions,
InputTypes extends object,
OutputTypes extends MetadataBearer,
ResolvedClientConfiguration,
> {
<InputType extends InputTypes, OutputType extends OutputTypes>(
command: Command<InputTypes, InputType, OutputTypes, OutputType, ResolvedClientConfiguration>,
command: CommandIO<InputType, OutputType>,
options?: HttpHandlerOptions
): Promise<Transform<OutputType, StreamingBlobPayloadOutputTypes | undefined, NarrowType>>;
<InputType extends InputTypes, OutputType extends OutputTypes>(
command: Command<InputTypes, InputType, OutputTypes, OutputType, ResolvedClientConfiguration>,
command: CommandIO<InputType, OutputType>,
cb: (err: unknown, data?: Transform<OutputType, StreamingBlobPayloadOutputTypes | undefined, NarrowType>) => void
): void;
<InputType extends InputTypes, OutputType extends OutputTypes>(
command: Command<InputTypes, InputType, OutputTypes, OutputType, ResolvedClientConfiguration>,
command: CommandIO<InputType, OutputType>,
options: HttpHandlerOptions,
cb: (err: unknown, data?: Transform<OutputType, StreamingBlobPayloadOutputTypes | undefined, NarrowType>) => void
): void;
<InputType extends InputTypes, OutputType extends OutputTypes>(
command: Command<InputTypes, InputType, OutputTypes, OutputType, ResolvedClientConfiguration>,
command: CommandIO<InputType, OutputType>,
options?: HttpHandlerOptions,
cb?: (err: unknown, data?: Transform<OutputType, StreamingBlobPayloadOutputTypes | undefined, NarrowType>) => void
): Promise<Transform<OutputType, StreamingBlobPayloadOutputTypes | undefined, NarrowType>> | void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -87,7 +88,7 @@ interface MyClient extends Client<MyInput, MyOutput, MyConfig> {
{
interface NodeJsMyClient extends NodeJsClient<MyClient> {}
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<MyInput, MyOutput>, { abortSignal: null as any });

type A = Awaited<ReturnType<typeof sendCall>>;
type B = Omit<MyOutput, "body"> & { body?: SdkStream<IncomingMessage> };
Expand Down
42 changes: 22 additions & 20 deletions packages/types/src/transform/client-payload-blob-type-narrow.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
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 {
BrowserRuntimeStreamingBlobPayloadInputTypes,
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";

/**
Expand Down Expand Up @@ -80,12 +82,15 @@ export type BrowserXhrClient<ClientType extends object> = NarrowPayloadBlobTypes
*/
export type NarrowPayloadBlobOutputType<T, ClientType extends object> = {
[key in keyof ClientType]: [ClientType[key]] extends [
InvokeFunction<infer InputTypes, infer OutputTypes, infer ConfigType>,
InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>,
]
? NarrowedInvokeFunction<T, HttpHandlerOptions, InputTypes, OutputTypes, ConfigType>
: [ClientType[key]] extends [InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>]
? NarrowedInvokeMethod<T, HttpHandlerOptions, FunctionInputTypes, FunctionOutputTypes>
: ClientType[key];
? NarrowedInvokeMethod<T, HttpHandlerOptions, FunctionInputTypes, FunctionOutputTypes>
: ClientType[key];
} & {
send<Command>(
command: Command,
options?: any
): Promise<Transform<GetOutputType<Command>, StreamingBlobPayloadOutputTypes | undefined, T>>;
};

/**
Expand All @@ -95,21 +100,18 @@ export type NarrowPayloadBlobOutputType<T, ClientType extends object> = {
*/
export type NarrowPayloadBlobTypes<I, O, ClientType extends object> = {
[key in keyof ClientType]: [ClientType[key]] extends [
InvokeFunction<infer InputTypes, infer OutputTypes, infer ConfigType>,
InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>,
]
? NarrowedInvokeFunction<
? NarrowedInvokeMethod<
O,
HttpHandlerOptions,
Transform<InputTypes, StreamingBlobPayloadInputTypes | undefined, I>,
OutputTypes,
ConfigType
Transform<FunctionInputTypes, StreamingBlobPayloadInputTypes | undefined, I>,
FunctionOutputTypes
>
: [ClientType[key]] extends [InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>]
? NarrowedInvokeMethod<
O,
HttpHandlerOptions,
Transform<FunctionInputTypes, StreamingBlobPayloadInputTypes | undefined, I>,
FunctionOutputTypes
>
: ClientType[key];
: ClientType[key];
} & {
send<Command>(
command: Command,
options?: any
): Promise<Transform<GetOutputType<Command>, StreamingBlobPayloadOutputTypes | undefined, O>>;
};
17 changes: 17 additions & 0 deletions packages/types/src/transform/no-undefined.spec.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -114,4 +115,20 @@ type A = {
const assert6: Exact<typeof output.r.c, string | number | undefined> = true as const;
}
}

{
// Works with outputs of the "send" method.
const c = null as unknown as AssertiveClient<MyClient>;
const list = c.send(null as unknown as CommandIO<MyInput, MyOutput>);
const output = null as unknown as Awaited<typeof list>;

const assert1: Exact<typeof output.a, string | undefined> = true as const;
const assert2: Exact<typeof output.b, number | undefined> = true as const;
const assert3: Exact<typeof output.c, string | number | undefined> = true as const;
if (output.r) {
const assert4: Exact<typeof output.r.a, string | undefined> = true as const;
const assert5: Exact<typeof output.r.b, number | undefined> = true as const;
const assert6: Exact<typeof output.r.c, string | number | undefined> = true as const;
}
}
}
23 changes: 12 additions & 11 deletions packages/types/src/transform/no-undefined.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { InvokeFunction, InvokeMethod, InvokeMethodOptionalArgs } from "../client";
import type { InvokeMethod, InvokeMethodOptionalArgs } from "../client";
import type { GetOutputType } from "../command";

/**
* @public
Expand Down Expand Up @@ -62,11 +63,11 @@ type NarrowClientIOTypes<ClientType extends object> = {
InvokeMethodOptionalArgs<infer FunctionInputTypes, infer FunctionOutputTypes>,
]
? InvokeMethodOptionalArgs<NoUndefined<FunctionInputTypes>, NoUndefined<FunctionOutputTypes>>
: [ClientType[key]] extends [InvokeFunction<infer InputTypes, infer OutputTypes, infer ConfigType>]
? InvokeFunction<NoUndefined<InputTypes>, NoUndefined<OutputTypes>, ConfigType>
: [ClientType[key]] extends [InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>]
? InvokeMethod<NoUndefined<FunctionInputTypes>, NoUndefined<FunctionOutputTypes>>
: ClientType[key];
: [ClientType[key]] extends [InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>]
? InvokeMethod<NoUndefined<FunctionInputTypes>, NoUndefined<FunctionOutputTypes>>
: ClientType[key];
} & {
send<Command>(command: Command, options?: any): Promise<NoUndefined<GetOutputType<Command>>>;
};

/**
Expand All @@ -79,9 +80,9 @@ type UncheckedClientOutputTypes<ClientType extends object> = {
InvokeMethodOptionalArgs<infer FunctionInputTypes, infer FunctionOutputTypes>,
]
? InvokeMethodOptionalArgs<NoUndefined<FunctionInputTypes>, RecursiveRequired<FunctionOutputTypes>>
: [ClientType[key]] extends [InvokeFunction<infer InputTypes, infer OutputTypes, infer ConfigType>]
? InvokeFunction<NoUndefined<InputTypes>, RecursiveRequired<OutputTypes>, ConfigType>
: [ClientType[key]] extends [InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>]
? InvokeMethod<NoUndefined<FunctionInputTypes>, RecursiveRequired<FunctionOutputTypes>>
: ClientType[key];
: [ClientType[key]] extends [InvokeMethod<infer FunctionInputTypes, infer FunctionOutputTypes>]
? InvokeMethod<NoUndefined<FunctionInputTypes>, RecursiveRequired<FunctionOutputTypes>>
: ClientType[key];
} & {
send<Command>(command: Command, options?: any): Promise<RecursiveRequired<NoUndefined<GetOutputType<Command>>>>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"));
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 0067b9d

Please sign in to comment.