Members on models are always type or undefined

[jabalsad]

Hello,

The generated typescript types for structures have members that are always either the type or undefined.

E.g.

export interface Metadata {
  id: string | undefined;
  createdAt: Date | undefined;
  updatedAt: Date | undefined;
  deleted: boolean | undefined;
  lastUpdatedBy: string | undefined;
}

Even though these members have a @required trait applied to them. Can you elaborate a bit on the motivation here and if there is a way to prevent this from happening?

[adamthom-amzn]

It's been considered backwards compatible for Smithy services to remove @required. If we didn't include undefined in the type, client applications would have to make a code change to compensate when a service dropped @required.

We know this is a very annoying thing to have to work around in code. We have a proposal for a trait that allows this change to be made backwards-compatibly without including undefined (by falling back to the default value for the type). Unfortunately the TypeScript client code generator does not generate classes/constructors for types (as a size optimization), so there's no clear way for us to include a default, non-null value for members, so there may be no way to adopt this proposal backwards-compatibly when it lands.

[agacek]

This unfortunately makes Smithy SDKs tedious to use. For those of us willing to sacrifice backwards compatibility, could there be a flag to allow the more precise SDK generation?

[pcholakov]

It's been considered backwards compatible for Smithy services to remove @required.

This policy only makes sense on input shapes; it's the inverse when it comes to outputs – introducing @required is backwards-compatible, removing it is a breaking change. Is the input vs. output delineation something that we should be factoring in? I suspect that most users find the ... | undefined shapes a pain to work with on the response end of generated code.

To that end, I wonder if @eduardomourar's PR #566 wouldn't be substantially easier to accept if it was limited in scope to just output structures. Am I missing something?

[anderha]

I discovered this behavior while evaluating whether to use the code generator. Which, if I read the openapi and the smithy requirements correctly, seems to be a bug.

• Openapi distinguishes between required and nullable (ref: https://swagger.io/docs/specification/data-models/data-types/)
  • required: "By default, all object properties are optional. You can specify the required properties"
  • nullable: "OpenAPI 3.0 does not have an explicit null type as in JSON Schema, but you can use nullable: true to specify that the value may be null" and "If the null value needs to be allowed, add nullable: true"
• Smithy also distinguishes between required and nullable
  • nullable was implemented in may of this year: Add @nullable trait and update proposal smithy#1215

As of my understanding you need to declare a value as nullable in the smithy file otherwise it would be not nullable. Therefore I would expect the following behavior:

// .smithy file
structure Todo {
    id: String // -> not required and not nullable
}

// generated typescript
interface Todo {
  id?: string
}

// .smithy file
structure Todo {
    @required
    id: String // -> required and not nullable
}
// generated typescript
interface Todo {
    id: string
}

// .smithy file
structure Todo {
    @nullable
    id: String // -> not required and nullable
}
// generated typescript
interface Todo {
    id?: string | undefined
}

// .smithy file
structure Todo {
    @required
    @nullable
    id: String // -> required but nullable
}
// generated typescript
interface Todo {
    id: string | undefined
}

I would really appreciate if you could elaborate on this issue and the PRs #566 & #537 @adamthom-amzn @trivikr

[omerfarukaslan]

This is indeed an annoying issue. And the only thing that holds us from using Smithy. Our concern is that this could cause bugs in the future and it requires manual ugly work arounds.

Is there any update on this?

[cayman-amzn]

See #566

[gosar]

Mentioned this in the PR comments, but repeating it here for anyone looking to use this feature.

Wherever this flag is defined and documented, there should be some warning language calling out the risks of using this mode.

The risks are that when a server drops @required from an output shape (and replaced it with @default as per spec), if the server does not always serialize a value, customer code consuming the client and trying to access this member may get a NullPointerException. Smithy spec says

Authoritative model consumers like servers SHOULD always serialize default values to remove any ambiguity about the value of the most up to default value.

So one should use this mode on the client, only if the server is following this.

[gosar]

Resolved by PR #566

[gosar]

For the SSDK side, created issue #694