smithy-lang / smithy-typescript

Smithy code generators for TypeScript. (in development)
Apache License 2.0
221 stars 81 forks source link

Navigating to input and output types for a command in TypeScript is too tedious #1366

Closed jedwards1211 closed 2 days ago

jedwards1211 commented 1 month ago

The smithy-generated typedefs have several layers of indirection that make it tedious to navigate from a command to its input/output types.

For example, let's say I want to get to the input type for DescribeInstancesCommand:

  1. Cmd-Click on DescribeInstancesCommand
  2. I land on class DescribeInstancesCommand extends DescribeInstancesCommand_base { }
  3. Cmd-Click on DescribeInstancesCommand_base
  4. I land on const DescribeInstancesCommand_base with new (input: DescribeInstancesCommandInput)
  5. Cmd-Click on DescribeInstancesCommandInput
  6. I land on interface DescribeInstancesCommandInput extends DescribeInstancesRequest { }
  7. Cmd-Click on DescribeInstancesRequest
  8. Finally I get to the type I want to see

That's four Cmd-Clicks; after working with the TypeScript SDK for awhile, this has become persistently annoying. I'd rather it took just two Cmd-Clicks: one to get to the DescribeInstancesCommand declaration, and one to get to the type containing all of the input or output properties.

All of the AWS SDK Commands I've dealt with have a pattern like this:

export interface DescribeInstancesCommandInput extends DescribeInstancesRequest {
}
export interface DescribeInstancesCommandOutput extends DescribeInstancesResult, __MetadataBearer {
}
declare const DescribeInstancesCommand_base: {
    new (input: DescribeInstancesCommandInput): import("@smithy/smithy-client").CommandImpl<DescribeInstancesCommandInput, DescribeInstancesCommandOutput, EC2ClientResolvedConfig, ServiceInputTypes, ServiceOutputTypes>;
    new (...[input]: [] | [DescribeInstancesCommandInput]): import("@smithy/smithy-client").CommandImpl<DescribeInstancesCommandInput, DescribeInstancesCommandOutput, EC2ClientResolvedConfig, ServiceInputTypes, ServiceOutputTypes>;
    getEndpointParameterInstructions(): import("@smithy/middleware-endpoint").EndpointParameterInstructions;
};
export declare class DescribeInstancesCommand extends DescribeInstancesCommand_base {
}

I don't know what purpose the distinction between *Command/*Command_base or *CommandInput/*Request or *CommandOutput/*Result was designed for, but it's hard to understand what benefits it has in practice, that would outweigh the drawbacks of cumbersome navigation.

The following would be better for us to work with, and AFAIK have the same types:


export { DescribeInstancesRequest as DescribeInstancesCommandInput } from '../models/models_4'
export interface DescribeInstancesCommandOutput extends DescribeInstancesResult, __MetadataBearer {
    // really, DescribeInstancesResult might as well extend __MetadataBearer itself
    // and get re-exported as DescribeInstancesCommandOutput here
}

export declare class DescribeInstancesCommand {
    constructor (input: DescribeInstancesRequest): import("@smithy/smithy-client").CommandImpl<DescribeInstancesRequest, DescribeInstancesResult & __MetadataBearer, EC2ClientResolvedConfig, ServiceInputTypes, ServiceOutputTypes>;
    constructor (...[input]: [] | [DescribeInstancesRequest]): import("@smithy/smithy-client").CommandImpl<DescribeInstancesRequest, DescribeInstancesResult & __MetadataBearer, EC2ClientResolvedConfig, ServiceInputTypes, ServiceOutputTypes>;
    static getEndpointParameterInstructions(): import("@smithy/middleware-endpoint").EndpointParameterInstructions;
}

export { DescribeInstancesCommand as DescribeInstancesCommand_base }

At the very least, it would be helpful to have links to the *Request/*Result at the very bottom of the docstring for *Command:

/**
 *
 * ... lots of docstring
 *
 * @see {@link DescribeInstancesRequest} for command's raw `input` shape.
 * @see {@link DescribeInstancesResult} for command's raw `response` shape.
 */
export declare class DescribeInstancesCommand extends DescribeInstancesCommand_base {
}

Putting them at the bottom would ensure we don't have to scroll up to find them after jumping to the *Command type.

kuhe commented 3 weeks ago

We don't plan to change the order of the docstrings at this time.

terminology:

To reach the modeled type faster:

  1. Go-to-definition on any XCommand class.
  2. Scroll to the top of the file. Line 15 usually has the input and line 22 usually has the output. In VSCode I can use Cmd + Up.
  3. Go-to-definition on the base type of the XCommandInput or XCommandOutput types.

Alternative:

  1. Autocomplete any field of the input object.
  2. Go-to-definition on the field.

Why do the XCommandInput/Output types exists?

const input: XCommandInput = {}; const output: XCommandOutput = await client.send(new XCommand(input));


- To allow modifications. The SDK transforms certain modeled types to make them easier to use. For example, in the AWS SDK S3 Client, the output type is 
```ts
/**
 * @public
 *
 * The output of {@link GetObjectCommand}.
 */
export interface GetObjectCommandOutput extends Omit<GetObjectOutput, "Body">, __MetadataBearer {
  Body?: StreamingBlobPayloadOutputTypes;
}

to attach additional methods on the Body field for transforming it to a string or other stream types.

jedwards1211 commented 3 weeks ago

Autocompleting a field is okay when I'm already typing out the command. But it seems like fairly often I want to look at the fields before I've started typing the call, and then it's an unpleasant dev experience. Cmd-Click on the command brings up the declaration selector thing so it's an additional step to open the file. Always feels like it breaks my flow. I guess with practice I'll get faster at it but I'm disappointed you don't want to invest in making it more direct.

jedwards1211 commented 3 weeks ago

Hmmm, maybe I'm wrong about the duplicate declaration selector, maybe that was happening while I was experimenting with restructuring the definitions?

Anyway, if you've ever dealt with something that felt petty to complain about, yet has bothered you enough that it would feel dissatisfying to accept that it's always going to be that way, this is one of those experiences for me.

kuhe commented 3 weeks ago

We try to support 3 ways to find information about SDK operation input and output types:

  1. In the IDE starting from the Command class.
  2. In the IDE starting from an operation method e.g. new S3({}).getObject({ ... });
  3. Accompanying application coding with the API docs site open. https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/ec2/command/DescribeInstancesCommand/

For option 1, which you are using, we may be able to move the types closer to the entry point. I'll open a PR with what I find.

jedwards1211 commented 3 weeks ago

Oh nice, I didn't think to propose static field declarations. I think that would help me, thank you!

kuhe commented 2 weeks ago

This will take some time to deploy (if approved) due to the review process and other higher priority issues.

jedwards1211 commented 2 weeks ago

Okay makes sense, thanks :)

kuhe commented 2 days ago

closed the issue but it's pending a code generation update in https://github.com/aws/aws-sdk-js-v3 which may take another week or so

jedwards1211 commented 2 days ago

awesome, thanks!