aws-sdk-js-v3: Why is everything possibly undefined? (& other typing/doc questions)

I’ve been trying to work out how some of these things are intended to be used and having trouble.

Why are all fields possibly undefined?

eg (doc comments removed:

export interface Container {
    reason?: string;
    name?: string;
    exitCode?: number;
    // ...

Is the expectation that we should be putting in guard clauses for every API response, including ones that don’t return an error, for things that the APIs are expected to return?

How do we use <client>.config.* properties?

Some code naively ported from V2:

  const ecs = new ECS({});
  const clusters = await ecs.listClusters({});

  if (!clusters.clusterArns) {
    throw new CLIError(
      `Could not find any ECS clusters in ${ecs.config.region}`
    );
  }

ecs.config.region above is typed as Provider<string> | (string & Provider<string>) (which rightfully produces a warning with eslint about embedding it in a template string). Looking at Provider, it looks like it’s a promise wrapper. Are we supposed to check via typeof to see if it’s a function or a string, and await it if it’s a function?

Where are the SDK docs?

I’ve been relying on intellisense & manually reading the type definitions, but it’d be nice if AWS hosted a typedoc of the various packages.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 19
  • Comments: 17 (3 by maintainers)

Most upvoted comments

Ridiculous! Making all output fields optional so that they can remove them in future versions without breaking the API is just wrong. If the field is important, I will need to change my code when you stop sending the field - this is a breaking change in my app. By making important output fields optional you are actually making things worse because my code will still compile when you stop populating it!

I’m going to have to use a non-null assertion every time I use an AWS output field and my downstream code will just break if/when they stop sending it. I have no other choice (the only other option I have is to check whether an important field is null and it is, all I can do is throw an error).

Also chiming in, currently migrating to @aws-sdk/* and this is a MAJOR pain, why was this done? This almost seems like someone saw protobuff and only took the bad parts.

You segmented clients into smaller packages, if the api changes make breaking changes, don’t create completely arbitrary api constraints just so you can claim ‘technically not a breaking change 😃’ later.

So, coming back to this topic, I understand there is a rationale behind the current behavior (and changing it would be a breaking change).

How about having a flag that can be passed when doing a cdktf get (or cdktf.json or an environment var) so that it generates the TS with this other model people are asking for?

I just encountered this too, using EventBridge Scheduler and trying to update an existing schedule. As mentioned on the UpdateScheduleCommand page, it is recommended to fetch the schedule first with GetSchedule to avoid resetting to defaults on update. But in the output of GetScheduleCommand everything is optional, even the fields required on create/update. It is not even using the previously mentioned (equally questionable) pattern of “required with | undefined” as declared in the input types. Now i can’t just spread ...fetchedSchedule into the update input, but I could manually overwrite the required fields with the same possibly undefined value from fetchedSchedule, which would still fail if it actually was undefined… As everyone else mentioned, this is terrible!

Also, why does e.g. the UpdateScheduleCommand page show which input fields are required, but the linked UpdateScheduleCommandInput page doesn’t?

Ridiculous! Making all output fields optional so that they can remove them in future versions without breaking the API is just wrong. If the field is important, I will need to change my code when you stop sending the field

As an extension of this, This is the whole point in using versioning in the first place. As consumers we lock to particular versions to ensure compatibility. When we upgrade we acknowledge that the code we are using may change and that our code will need to change to accomodate it. By saying “well we may change the code in the future” you may as well save yourselves the trouble and just distribute a single version of the code as theres no benefit to having versioning behind this reasoning.

Your even including fields that are inherently required by the backend for it to be able to function, which doesnt make sense. My example being the ListTagsForResourceCommandInput. That function has a requirement for a resource arn parameter, even though the input parameter to be undefined. Pretty much defeats the purpose of a strongly typed language. The DescribeLogGroupsCommandOutput even returns a LogGroup array where even field is optional. The backend cant function without having an ARN, so why are we expected to validate its existense.

I’ve raised this issue myself before, and was told:

Using | undefined is a thought out decision explained in #1124 (comment)

I’ve come across this issue as well while using the textract client.

For example:

export interface BoundingBox {
    Width?: number;
    Height?: number;
    Left?: number;
    Top?: number;
}

That object cannot logically exist without all those fields defined.

If you’re using typescript this causes code that is much more unnecessarily verbose, having to check all the members for undefined.

Which seems to come from here: https://github.com/awslabs/smithy-typescript/blob/master/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java#L64

String optionalSuffix = shape.isUnionShape() || !isRequiredMember(member) ? "?" : "";
String typeSuffix = isRequiredMember(member) ? " | undefined" : "";

It’s really not convenient the way it works right now, e.g.:

// given this (where Handler is @required and Code is optional)
type CreateFunctionRequest = {
  Handler?: string;
  Code: FunctionCode | undefined;
};
// you can do this:
const test: CreateFunctionRequest = {
  Code: undefined
};

And even if you pass Handler, you cannot omit Code and have to explicitly set it to undefined.