amplify-js icon indicating copy to clipboard operation
amplify-js copied to clipboard

Update types for Promises on Auth calls

Open jhechtf opened this issue 3 years ago • 3 comments

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.

jhechtf avatar Nov 27 '21 04:11 jhechtf

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!

sammartinez avatar Nov 29 '21 23:11 sammartinez

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

jhechtf avatar Nov 30 '21 01:11 jhechtf

Any word on this @sammartinez ?

jhechtf avatar Sep 10 '22 23:09 jhechtf

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!

mrcoles avatar Mar 02 '23 02:03 mrcoles

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

jimblanc avatar Mar 23 '23 15:03 jimblanc

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 avatar Oct 03 '23 20:10 cwomack

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 avatar Nov 16 '23 21:11 cwomack