amazon-archives / aws-cognito-angular-quickstart

An Angular(v5)-based QuickStart single-page app utilizing Amazon Cognito, S3, and DynamoDB (Serverless architecture)
https://cognito.budilov.com
Apache License 2.0
689 stars 302 forks source link

Refactor typescript bindings #77

Closed dpantke closed 7 years ago

dpantke commented 7 years ago

This a relatively moderate refactor to make the example more "Angular-esque" by switching from the JS libraries to using the Typescript bindings for both the AWS SDK and the Cognito Identity SDK. This resulted the disabling of the Mobile Analytics client for now.

dpantke commented 7 years ago

The problem with the previous iteration of this PR had to do with some more circular processing problems in the initial authentication (callbacks make this more non-obvious). In my testing, I still had session state in the browser that I had based on previous behavior, so I wasn't hitting the code branch dealing with a virgin browser. This has been corrected, and I validated a good chunk of the functionality using a cleansed Chrome Incognito window.

KoldBrewEd commented 7 years ago

In my experience the typings for the general AWS SDK worked great however not so much for the Cognito Identity SDK, it always returned missing dependency errors. I'm very interested to confirm this is working so I can update my own angular app.

dpantke commented 7 years ago

While there is a lot of refactor work ahead (the relationship between AwsUtil, CognitoUtil, and the rest of the app are strange and could use a chunk of abstraction work), this is about the smallest PR I could sneak in to get things running, and they should be running reasonably. I need to find some AWS service mocks so I can get unit tests running.... :)

vbudilov commented 7 years ago

Did you check in all of the code?

dpantke commented 7 years ago

So I'm doing most of my testing via "npm start". The S3Service is orphaned in the master branch right now, so I think that the development test environment is "as needed" loading classes, whereby your build may be compiling all TS regardless of references. Regardless, I've propogated my refactor to that file as well.

vbudilov commented 7 years ago

Very close, but the DDB functionality isn't working, and I'm still getting the following exception: ERROR Error: Uncaught (in promise): TypeError: Cannot read property 'identityId' of undefined TypeError: Cannot read property 'identityId' of undefined at CognitoUtil.webpackJsonp.33.CognitoUtil.getCognitoIdentity (cognito.service.ts:85) at DynamoDBService.webpackJsonp.78.DynamoDBService.getLogEntries (ddb.service.ts:32) at UseractivityComponent.webpackJsonp.166.UseractivityComponent.isLoggedIn (useractivity.component.ts:31) at user-login.service.ts:116 at CognitoUser.getSession (CognitoUser.js:875) at UserLoginService.webpackJsonp.29.UserLoginService.isAuthenticated (user-login.service.ts:109) at new UseractivityComponent (useractivity.component.ts:22) at createClass (core.es5.js:10844) at createDirectiveInstance (core.es5.js:10675) at createViewNodes (core.es5.js:12024) at createRootView (core.es5.js:11929) at callWithDebugContext (core.es5.js:13060) at Object.debugCreateRootView [as createRootView] (core.es5.js:12521) at ComponentFactory.create (core.es5.js:9866) at ComponentFactoryBoundToModule.create (core.es5.js:3447) at CognitoUtil.webpackJsonp.33.CognitoUtil.getCognitoIdentity (cognito.service.ts:85) at DynamoDBService.webpackJsonp.78.DynamoDBService.getLogEntries (ddb.service.ts:32) at UseractivityComponent.webpackJsonp.166.UseractivityComponent.isLoggedIn (useractivity.component.ts:31) at user-login.service.ts:116 at CognitoUser.getSession (CognitoUser.js:875) at UserLoginService.webpackJsonp.29.UserLoginService.isAuthenticated (user-login.service.ts:109) at new UseractivityComponent (useractivity.component.ts:22) at createClass (core.es5.js:10844) at createDirectiveInstance (core.es5.js:10675) at createViewNodes (core.es5.js:12024) at createRootView (core.es5.js:11929) at callWithDebugContext (core.es5.js:13060) at Object.debugCreateRootView [as createRootView] (core.es5.js:12521) at ComponentFactory.create (core.es5.js:9866) at ComponentFactoryBoundToModule.create (core.es5.js:3447) at resolvePromise (zone.js:712) [angular] at resolvePromise (zone.js:683) [angular] at vendor.bundle.js:112838:17 [angular] at Object.onInvokeTask (core.es5.js:4136) [angular] at ZoneDelegate.invokeTask (zone.js:397) [angular] at Zone.runTask (zone.js:165) [ => angular] at drainMicroTaskQueue (zone.js:593) [] at HTMLAnchorElement.ZoneTask.invoke (zone.js:464) [] defaultErrorLogger @ core.es5.js:1085 ErrorHandler.handleError @ core.es5.js:1145 next @ core.es5.js:4774 schedulerFn @ core.es5.js:3848 SafeSubscriber.__tryOrUnsub @ Subscriber.js:234 SafeSubscriber.next @ Subscriber.js:183 Subscriber._next @ Subscriber.js:125 Subscriber.next @ Subscriber.js:89 Subject.next @ Subject.js:55 EventEmitter.emit @ core.es5.js:3834 NgZone.triggerError @ core.es5.js:4205 onHandleError @ core.es5.js:4166 ZoneDelegate.handleError @ zone.js:369 Zone.runGuarded @ zone.js:141 _loop_1 @ zone.js:604 drainMicroTaskQueue @ zone.js:613 ZoneTask.invoke @ zone.js:464

vbudilov commented 7 years ago

Running npm start is a good start, but you might need to login to the webpage and see if you can bring up all of the pages. Since I have my settings there, you don't have to setup your AWS environment -- everything runs in mine.

dpantke commented 7 years ago

There are lots of interesting code paths to traverse here, to repro I actually had to:

Investigating the new error. BTW if you refresh the page you hit the code path that makes it all work.

vbudilov commented 7 years ago

Interesting...I actually always start testing from an incognito window.

vbudilov commented 7 years ago

You're right...when I refresh the page everything starts working :)

dpantke commented 7 years ago

Alright, that took much longer than I thought, but I figured out how to kick things. Needs to be refactored to look better (All this credential work probably needs to be done outside the authenticateUser function), but it should be humming OK now.

vbudilov commented 7 years ago

Thanks for the PR @dpantke -- works great!