aws-sdk-js-v3
aws-sdk-js-v3 copied to clipboard
Every field in DescribeOrganizationResponse is optional
Describe the bug
The typings for DescribeOrganizationResponse has every field specified as optional (using the ?: syntax). This makes the following code not compile with TypeScript:
import { WorkMail } from '@aws-sdk/client-workmail'
const workmail = new WorkMail({ region: 'eu-west-1' })
const org = await workmail.describeOrganization({ OrganizationId: 'example' })
// ERROR: `org.OrganizationId` can be undefined
const id: string = org.OrganizationId
This is a minimal example and it's of course easy to work around. But my actual use case involve multiple functions that takes organization: DescribeOrganizationResponse as a parameter.
Expected Behavior
I expected at the very least OrganizationId to be non-optional. And I think that ARN, Alias and State should be as well.
Current Behavior
They are optional, which forces me to either add special error handling that should never trigger, or silence the TypeScript compiler using @ts-expect-error.
Reproduction Steps
import { WorkMail } from '@aws-sdk/client-workmail'
const workmail = new WorkMail({ region: 'eu-west-1' })
const org = await workmail.describeOrganization({ OrganizationId: 'example' })
// ERROR: `org.OrganizationId` can be undefined
const id: string = org.OrganizationId
Possible Solution
Remove the ? for the affected fields:
--- a.ts 2024-01-18 15:58:53
+++ b.ts 2024-01-18 15:59:31
@@ -1826,17 +1826,17 @@
* @public
* <p>The identifier of an organization.</p>
*/
- OrganizationId?: string;
+ OrganizationId: string;
/**
* @public
* <p>The alias for an organization.</p>
*/
- Alias?: string;
+ Alias: string;
/**
* @public
* <p>The state of an organization.</p>
*/
- State?: string;
+ State: string;
/**
* @public
* <p>The identifier for the directory associated with an WorkMail organization.</p>
@@ -1868,7 +1868,7 @@
* @public
* <p>The Amazon Resource Name (ARN) of the organization.</p>
*/
- ARN?: string;
+ ARN: string;
/**
* @public
* <p>The user ID of the migration admin if migration is enabled for the organization.</p>
Additional Information/Context
No response
SDK version used
3.490.0
Environment details (OS name and version, etc.)
macOS 14, Node.js 20.9.0
Similarly, Id on User is also optional:
import { WorkMail } from '@aws-sdk/client-workmail'
const workmail = new WorkMail({ region: 'eu-west-1' })
const list = await workmail.listUsers({ OrganizationId: 'example' })
for (const user of list.Users) {
// ERROR: `user.Id` can be undefined
const id: string = user.Id
}
(and also Users on ListUsersResponse)
Hi there @LinusU - thanks for reaching out.
I was able to confirm that OrganizationId is required param which is also documented in our JavaScript SDK docs and it's working as expected. Can you elaborate more on your statement of "They are optional, which forces me to either add special error handling that should never trigger, or silence the TypeScript compiler using @ts-expect-error."?
@aBurmeseDev hello, thanks for taking a look at this.
I did not mean in the request parameters, but rather in the response.
That is, in the value I have after called and awaited workmail.describeOrganization, the OrganizationId is typed as optional.
If you look at the code snippet under reproduction step, you can see that I cannot assign org.OrganizationId to something that expects a string.
Similarly, after doing listUsers, the Id inside the returned array is also typed as optional, which makes it very hard to work with.
This seems to affect almost every part of the WorkMail API types, some examples:
createOrganizationhasOrganizationIdon the response as optional: https://github.com/aws/aws-sdk-js-v3/blob/40442fba7e910ad679be583e83cb375bd51d5b15/clients/client-workmail/src/models/models_0.ts#L1150-L1156descibeOrganizationhasOrganizationId,Alias, andStateon the response as optional: https://github.com/aws/aws-sdk-js-v3/blob/40442fba7e910ad679be583e83cb375bd51d5b15/clients/client-workmail/src/models/models_0.ts#L2197-L2214createUserhasUserIdon the response as optional: https://github.com/aws/aws-sdk-js-v3/blob/40442fba7e910ad679be583e83cb375bd51d5b15/clients/client-workmail/src/models/models_0.ts#L1315-L1320listUsershasIdon the users list as optional: https://github.com/aws/aws-sdk-js-v3/blob/40442fba7e910ad679be583e83cb375bd51d5b15/clients/client-workmail/src/models/models_0.ts#L4639-L4644
Similarly, in some input types the fields are typed as required but that they can contain undefined. e.g. the types allow calling registerToWorkMail like so:
await workmail.registerToWorkMail({
Email: undefined,
EntityId: undefined,
OrganizationId: undefined
})
https://github.com/aws/aws-sdk-js-v3/blob/40442fba7e910ad679be583e83cb375bd51d5b15/clients/client-workmail/src/models/models_0.ts#L5034-L5062
Hello @aBurmeseDev, did you get a chance to look further into this? Is there anything that I can do to help?
Hi @LinusU - thanks for clarifying additional details and apology for the delay. I reached out to service team to get their thoughts on this and haven't heard back but I followed up on it. I will update here once I hear back. P123189247
@aBurmeseDev - Hello! Did you receive an update from the services team? Thanks
ping @aBurmeseDev - any updates? or anything that I can do from my side in order to help move this forward? 🙏
Hi @LinusU - apologies for the delay. Service team is aware of this, however I can't say where this will end up on their priority unless it's a bug. If you look at other services like S3 listBucketsOutput, they're also optional: https://github.com/aws/aws-sdk-js-v3/blob/40442fba7e910ad679be583e83cb375bd51d5b15/clients/client-s3/src/models/models_0.ts#L11451-L11463
Also this's out of our (SDK) control as you can see here in smithy model where service API doesn't enforce it to be required: https://github.com/aws/aws-sdk-js-v3/blob/88213abea034a20c1ea1c91906a07916beffd7fb/codegen/sdk-codegen/aws-models/workmail.json#L2740-L2748
With that said, I will post back here when I hear more from service team or you can directly reach out to them via AWS Support.
Thank you for keeping me informed. I feel that this impacts the ergonomics quite a bit so it would be great to have it fixed. Will stay notified for further updates!