fix(authenticator): fetch current user on routed apps after refresh #1580

wlee221 · 2022-03-24T12:24:59Z

Problem

Previously, useAuthenticator hook/composable/service required an instance of Authenticator in the DOM tree to initiate the machine and look for any current authenticated user in the local storage.

This caused problems when a user refreshed in an authenticated route. For example, imagine the following structure:

/ is the entry to the home page and has the <Authenticator /> instance to enable sign in.
/my-account is an authenticated route that users can go after they signed in. This doesn't have <Authenticator /> in DOM because they already signed in.

Whenever a user refreshed on /my-account, the page wouldn't have an Authenticator in the DOM to initiate the authMachine, and so auth contexts from the hooks returned undefined.

Fix

This PR adjusts useAuthenticator hooks so that it first looks for Auth.currentAuthenticatedUser. If there are, it'll just transition directly to authenticated route and does not require an instance of <Authenticator /> whatsoever. Otherwise, it would proceed to the setup state, in which it will wait for Authenticator to send the INIT event.

From the framework side, <Authenticator /> will subscribe to authMachine changes. Once the machine gets to setup state and is ready for the INIT event, authenticator will send an INIT event with all the machine options.

Remaining Problems

Note that there are still bugs we have to address for routed app use cases, namely #1581. This PR will at least correct the refresh behavior and remove the need for Authenticator in the DOM tree for machines to start.

Issue #, if available

Related to #1332, #1497

Description of how you validated changes

#1578 adds new examples and tests for refreshing. #1578 is to be merged into this PR once approved.

Checklist

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

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

changeset-bot · 2022-03-24T12:25:01Z

🦋 Changeset detected

Latest commit: 111bf94

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@aws-amplify/ui-react	Patch
@aws-amplify/ui	Patch
@aws-amplify/ui-vue	Patch
@aws-amplify/ui-angular	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This reverts commit df53b24.

lgtm-com · 2022-03-24T12:36:44Z

This pull request introduces 2 alerts when merging fc71b28 into eedae23 - view on LGTM.com

new alerts:

2 for Unused variable, import, function or class

wlee221 · 2022-03-24T13:17:47Z

packages/ui/src/machines/authenticator/index.ts

@@ -25,47 +25,58 @@ export function createAuthenticatorMachine() {
      context: {
        user: undefined,
        config: {},
-        services: {},
+        services: { ...defaultServices },
        actorRef: undefined,
      },
      states: {
        // See: https://xstate.js.org/docs/guides/communication.html#invoking-promises
        idle: {


idle state now starts with Auth.currentAuthenticatedUser call.

Knowing that, idle is a bad name (it's on me!) and should be named more explicitly. I scoped this out of this PR, because that would change our route names and I would rather deal with it in 3.0 changes.

wlee221 · 2022-03-24T13:18:46Z

packages/ui/src/machines/authenticator/index.ts

-                target: 'authenticated',
+          initial: 'waitConfig',
+          states: {
+            waitConfig: {


At this stage, we confirmed that no user has signed in yet. Now we wait for Authenticator to send INIT event to pass machine props like initialState.

wlee221 · 2022-03-24T13:19:28Z

packages/ui/src/machines/authenticator/index.ts

              },
-              onError: [
+            },
+            applyConfig: {


Once we have all the machine props ready, we apply those configs to get loginMechanisms, etc.

wlee221 · 2022-03-24T13:22:11Z

...angular/src/lib/components/authenticator/components/authenticator/authenticator.component.ts

+    // send INIT event once machine is at 'setup' state
+    this.authenticator.subscribe(() => {
+      const { route } = this.authenticator;
+      if (!this.hasInitialized && route === 'setup') {


Previously authMachine got stuck in idle state until an Authenticator in DOM sends an INIT event on mount. Now authMachine starts independently (see state machine changes below), and will explicitly ask for config from Authenticator once it's in setup state.

Once Authenticator sees that state is in setup, it'll send the usual INIT event.

calebpollman

Granted I am still learning how xstate and the Authenticator state machine work but LGTM 👍🏽. Left some questions

packages/ui/src/machines/authenticator/index.ts

packages/vue/src/components/authenticator.vue

packages/react/src/components/Authenticator/Authenticator.tsx

packages/vue/src/components/authenticator.vue

Co-authored-by: Caleb Pollman <cpollman@amazon.com>

…plify/amplify-ui into get-user-on-refresh/state-machine

calebpollman

LGTM 🚢 Thanks for addressing the feedback!

* Angular Example * React example * Vue examples * Add missing ! * Update useAuthenticator test * Use slots on parent route * Configure Amplify on `/home` * Add react sign out button * Update wording on react example * Add sign out test * Add angular sign out buttons * Sign Out button in Vue * destructure signOut ref * Add hidden Authenticator on Vue * Fix copy paste typo

lgtm-com · 2022-03-24T23:38:22Z

This pull request introduces 3 alerts when merging 43119e4 into 8278c72 - view on LGTM.com

new alerts:

3 for Unused variable, import, function or class

…plify/amplify-ui into get-user-on-refresh/state-machine

wlee221 · 2022-03-25T00:19:46Z

fyi, I added 9 liner commit b8c36bb to fix a autoSignIn test failure. autoSignIn flow needed to be adjusted so that it goes to the correct state based on signUp result. This uses the same flow as before, just made more explicit.

I'll merge this tomorrow morning unless there are objections!

lgtm-com · 2022-03-25T17:22:46Z

This pull request introduces 3 alerts when merging 111bf94 into 683eac9 - view on LGTM.com

new alerts:

3 for Unused variable, import, function or class

slaymance

Looks good to me!

wlee221 added 2 commits March 23, 2022 22:53

[Revert] use devTools

df53b24

Check for current user first before init

7e00926

wlee221 added 4 commits March 24, 2022 05:25

Revert "[Revert] use devTools"

dbe5256

This reverts commit df53b24.

react: send INIT event on 'setup' state

0688fc4

vue: send INIT event on 'setup' state

0b2a066

angular: send INIT event on 'setup' state

fc71b28

wlee221 changed the title ~~Get user on refresh/state machine~~ fix(authenticator): get current user on refresh on routed apps Mar 24, 2022

wlee221 had a problem deploying to ci March 24, 2022 12:39 Failure

wlee221 had a problem deploying to ci March 24, 2022 12:39 Error

wlee221 temporarily deployed to ci March 24, 2022 12:39 Inactive

wlee221 mentioned this pull request Mar 24, 2022

fix(examples,e2e): Add examples and tests for #1580 #1578

Merged

7 tasks

wlee221 added 2 commits March 24, 2022 06:11

Remove unused var

4f086c9

Move comment

e777730

wlee221 commented Mar 24, 2022

View reviewed changes

wlee221 temporarily deployed to ci March 24, 2022 13:24 Inactive

wlee221 had a problem deploying to ci March 24, 2022 13:24 Failure

wlee221 had a problem deploying to ci March 24, 2022 13:24 Error

Create five-crabs-help.md

37588e0

wlee221 changed the title ~~fix(authenticator): get current user on refresh on routed apps~~ fix(authenticator): fetch current user on routed apps after refresh Mar 24, 2022

wlee221 temporarily deployed to ci March 24, 2022 13:49 Inactive

wlee221 had a problem deploying to ci March 24, 2022 13:49 Error

wlee221 had a problem deploying to ci March 24, 2022 13:49 Failure

calebpollman reviewed Mar 24, 2022

View reviewed changes

Update packages/ui/src/machines/authenticator/index.ts

487a9a3

Co-authored-by: Caleb Pollman <cpollman@amazon.com>

wlee221 temporarily deployed to ci March 24, 2022 18:05 Inactive

wlee221 added 3 commits March 24, 2022 15:35

Unsubscribe on vue

c3a7daf

Unsubscribe on angular

a72d2c3

Merge branch 'get-user-on-refresh/state-machine' of github.com:aws-am…

7857497

…plify/amplify-ui into get-user-on-refresh/state-machine

wlee221 requested a review from calebpollman March 24, 2022 22:43

calebpollman approved these changes Mar 24, 2022

View reviewed changes

wlee221 temporarily deployed to ci March 24, 2022 22:50 Inactive

wlee221 had a problem deploying to ci March 24, 2022 22:50 Failure

wlee221 had a problem deploying to ci March 24, 2022 22:50 Error

wlee221 temporarily deployed to ci March 24, 2022 23:43 Inactive

wlee221 had a problem deploying to ci March 24, 2022 23:43 Error

wlee221 had a problem deploying to ci March 24, 2022 23:43 Failure

wlee221 added 2 commits March 24, 2022 17:08

fix(e2e): AutoSignIn directly after signUp is done

b8c36bb

Merge branch 'get-user-on-refresh/state-machine' of github.com:aws-am…

8ce83b1

…plify/amplify-ui into get-user-on-refresh/state-machine

Trigger CI

111bf94

wlee221 temporarily deployed to ci March 25, 2022 17:27 Inactive

slaymance approved these changes Mar 25, 2022

View reviewed changes

wlee221 merged commit 1ac9cda into main Mar 25, 2022

wlee221 deleted the get-user-on-refresh/state-machine branch March 25, 2022 18:58

This was referenced Mar 25, 2022

Version Packages #1589

Merged

Version Packages calebpollman/amplify-ui#1

Closed

ErikCH mentioned this pull request Apr 28, 2022

Discussion: Use cases for useAuthenticator Hook #1497

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(authenticator): fetch current user on routed apps after refresh #1580

fix(authenticator): fetch current user on routed apps after refresh #1580

wlee221 commented Mar 24, 2022 •

edited

Loading

changeset-bot bot commented Mar 24, 2022 •

edited

Loading

lgtm-com bot commented Mar 24, 2022

wlee221 Mar 24, 2022

wlee221 Mar 24, 2022

wlee221 Mar 24, 2022

ErikCH Mar 24, 2022

wlee221 Mar 24, 2022

calebpollman left a comment

calebpollman left a comment

lgtm-com bot commented Mar 24, 2022

wlee221 commented Mar 25, 2022

lgtm-com bot commented Mar 25, 2022

slaymance left a comment

fix(authenticator): fetch current user on routed apps after refresh #1580

fix(authenticator): fetch current user on routed apps after refresh #1580

Conversation

wlee221 commented Mar 24, 2022 • edited Loading

Problem

Fix

Remaining Problems

Issue #, if available

Description of how you validated changes

Checklist

changeset-bot bot commented Mar 24, 2022 • edited Loading

🦋 Changeset detected

lgtm-com bot commented Mar 24, 2022

wlee221 Mar 24, 2022

Choose a reason for hiding this comment

wlee221 Mar 24, 2022

Choose a reason for hiding this comment

wlee221 Mar 24, 2022

Choose a reason for hiding this comment

ErikCH Mar 24, 2022

Choose a reason for hiding this comment

wlee221 Mar 24, 2022

Choose a reason for hiding this comment

calebpollman left a comment

Choose a reason for hiding this comment

calebpollman left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Mar 24, 2022

wlee221 commented Mar 25, 2022

lgtm-com bot commented Mar 25, 2022

slaymance left a comment

Choose a reason for hiding this comment

wlee221 commented Mar 24, 2022 •

edited

Loading

changeset-bot bot commented Mar 24, 2022 •

edited

Loading