Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update types for Promises on Auth calls #9286

Closed
3 tasks done
jhechtf opened this issue Nov 27, 2021 · 7 comments
Closed
3 tasks done

Update types for Promises on Auth calls #9286

jhechtf opened this issue Nov 27, 2021 · 7 comments
Labels
Auth Related to Auth components/category feature-request Request a new feature fixed-in-dev-preview Issues that are fixed in v6 dev preview TypeScript Related to TypeScript issues

Comments

@jhechtf
Copy link

jhechtf commented Nov 27, 2021

Before opening, please confirm:

JavaScript Framework

React, Not applicable

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

# Put output below this line
  System:
    OS: Windows 10 10.0.19042
    CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
    Memory: 5.41 GB / 15.87 GB
  Binaries:
    Node: 14.16.0 - C:\Program Files\nodejs\node.EXE       
    npm: 6.14.11 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (96.0.1054.34)
    Internet Explorer: 11.0.19041.1202
  npmGlobalPackages:
    pnpm: 5.18.3

Describe the bug

Note: I do not use the Amplify CLI, and I am only using the @aws-amplify/auth package directly. I am also currently using the following packages/versions

@aws-amplify/core: v4.3.8
@aws-amplify/auth: v4.3.16 (both most recent version as of writing this)

The TypeScript definitions for the Auth package heavily use the any as a return type when they should be denoted as some other type.

Expected behavior

The return types for the typescript definitions should entirely remove the use of any in the return types of the methods.

Reproduction steps

  1. Create new project.
  2. Install @aws-amplify/core and @aws-amplfiy/auth
  3. Configure the Auth appropriately to use an AWS Cognito User Pool.
  4. Attempt to use one of the many methods that has incorrect / insufficient type definitions associated.

Code Snippet

the project that got this all started is located at https://github.com/vini-vici/front-end

Some examples:

import Auth from '@aws-amplify/auth';
// Return type of Auth.signIn is `Promise<any>` when the result is `CognitoUser`
const value = await Auth.signIn('user','very secure password'); 

// Return type for Auth.currentUserInfo() is `Promise<any>` when it is more accurately conveyed as `{ username: string; attributes: Record<string, string | boolean>}`
const currentUser = await Auth.currentUserInfo();

// return type `Promise<any>`
const confirmSignup = await Auth.confirmSignUp('new user', 'new password');

// Return type `Promise<any>`
const signedOut = await Auth.signOut();

Log output

// Put your logs below this line

N/A

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

Currently the work around is to have to manually type the statements, e.g.

const user = await Auth.signIn(username, password) as CognitoUser;

This becomes harder in more obscure places where it's much harder to tell what the return result actually is, or if it has an associated type in the definitions.

@chrisbonifacio chrisbonifacio self-assigned this Nov 29, 2021
@chrisbonifacio chrisbonifacio added Auth Related to Auth components/category TypeScript Related to TypeScript issues pending-triage Issue is pending triage labels Nov 29, 2021
@sammartinez sammartinez added feature-request Request a new feature and removed pending-triage Issue is pending triage labels Nov 29, 2021
@sammartinez
Copy link
Contributor

Thanks for this feedback @jhechtf. I have marked this as a feature request as this is something we are currently evaluating for the Auth Category. While we don't have a timeline at this moment as to when to expect for this TypeScript work to be completed, we will keep you updated once we have an update on this front. Thanks!

@jhechtf
Copy link
Author

jhechtf commented Nov 30, 2021

Thanks; let me know if there is anything i can do to assist.

@evcodes evcodes self-assigned this Dec 2, 2021
@evcodes evcodes removed their assignment Feb 24, 2022
@jhechtf
Copy link
Author

jhechtf commented Sep 10, 2022

Any word on this @sammartinez ?

@mrcoles
Copy link
Contributor

mrcoles commented Mar 2, 2023

Upon reviewing the code in Auth.ts there are 25 instances of CognitoUser | any.

This type: CognitoUser | any is no different from any and is therefore not helpful at all.

These usecases should ideally just be CognitoUser or if necessary something like CognitoUser | null.

For example, Auth.currentAuthenticatedUser appears to raise an exception (rejecting the promise) whenever a user doesn’t exist, so the return type should just be Promise<CognitoUser>, but it requires updates like changing:

let user = null;
try {
	user = await this.currentUserPoolUser(params);
} catch (e) {
  
}
this.user = user;
return this.user;

to this:

try {
	this.user = await this.currentUserPoolUser(params);  // also this function needs updating typing
	return this.user;
} catch (e) {
  
}

This would be a great improvement on TypeScript support!

@jimblanc
Copy link
Contributor

We have published an RFC on our plan for improving TypeScript support in Amplify JS & would love to get your feedback & suggestions!

RFC: Amplify JS TypeScript Improvements

@cwomack
Copy link
Contributor

cwomack commented Oct 3, 2023

The developer preview for v6 of Amplify has officially been released with improved support for TypeScript and much more! Please check out our announcement and updated documentation to see what has changed.

This issue should be resolved within the dev preview and upcoming General Availability for Amplify v6, but let us know with a comment if there are further issues.

@cwomack cwomack added the fixed-in-dev-preview Issues that are fixed in v6 dev preview label Oct 3, 2023
@cwomack
Copy link
Contributor

cwomack commented Nov 16, 2023

With the release of the latest major version of Amplify (aws-amplify@>6), this issue should now be resolved! Please refer to our release announcement, migration guide, and documentation for more information.

@cwomack cwomack closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auth Related to Auth components/category feature-request Request a new feature fixed-in-dev-preview Issues that are fixed in v6 dev preview TypeScript Related to TypeScript issues
Projects
None yet
Development

No branches or pull requests

7 participants