Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(auth): update FetchDevicesOutput output type to include name attribute #14186

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

ashwinkumar6
Copy link
Member

@ashwinkumar6 ashwinkumar6 commented Feb 3, 2025

Description of changes

  • Add name attribute to fetchDevices API's return type FetchDevicesOutput
  • Update API to manually type the return object
    • Update unit test to catch such issues

Issue #, if available

#14178

Description of how you validated changes

Before
Screenshot 2025-02-03 at 2 36 46 PM
After
Screenshot 2025-02-03 at 2 37 14 PM

Checklist

  • PR description included
  • yarn test passes
  • Unit Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

Checklist for repo maintainers

  • Verify E2E tests for existing workflows are working as expected or add E2E tests for newly added workflows
  • New source file paths included in this PR have been added to CODEOWNERS, if appropriate

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -81,7 +81,23 @@ describe('fetchDevices', () => {
});

it('should fetch devices and parse client response correctly', async () => {
expect(await fetchDevices()).toEqual([apiOutputDevice]);
const {
Copy link
Member Author

@ashwinkumar6 ashwinkumar6 Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably update all API unit tests to

  • use a wrapper to catch input type issues
  • destructure result object to catch output type issues
    (reference)

@@ -77,7 +77,7 @@ const parseDevicesResponse = async (
{},
);

return {
const result: AWSAuthDevice = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could be

devices.map<AWSAuthDevice>()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm by this 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried this as well but TS only enforces mandatory keys

@ashwinkumar6 ashwinkumar6 merged commit 289f3e8 into aws-amplify:main Feb 4, 2025
30 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants