fix(singleton): allow oauth listener to be called with the latest config #13642
+120
−44
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,14 +13,14 @@ import { | |
| } from './types'; | ||
| import { AuthClass } from './Auth'; | ||
| import { ADD_OAUTH_LISTENER } from './constants'; | ||
| import { OAuthListener } from './Auth/types'; | ||
|
|
||
| export class AmplifyClass { | ||
| private oAuthListener: OAuthListener | undefined = undefined; | ||
|
|
||
| resourcesConfig: ResourcesConfig; | ||
| libraryOptions: LibraryOptions; | ||
| private listenerTimeout?: ReturnType<typeof setTimeout>; | ||
|
|
||
| /** | ||
| * Cross-category Auth utilities. | ||
|
|
@@ -53,7 +53,6 @@ export class AmplifyClass { | |
| libraryOptions?: LibraryOptions, | ||
| ): void { | ||
| const resolvedResourceConfig = parseAmplifyConfig(resourcesConfig); | ||
| this.resourcesConfig = resolvedResourceConfig; | ||
|
|
||
| if (libraryOptions) { | ||
|
|
@@ -75,7 +74,10 @@ export class AmplifyClass { | |
| AMPLIFY_SYMBOL, | ||
| ); | ||
|
|
||
| const config = this.resourcesConfig.Auth?.Cognito; | ||
| if (config?.loginWith?.oauth && this.oAuthListener) { | ||
| this.notifyOAuthListener(this, config, this.oAuthListener); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -98,17 +100,27 @@ export class AmplifyClass { | |
| } | ||
| } | ||
|
|
||
| private notifyOAuthListener( | ||
| amplify: AmplifyClass, | ||
| config: AuthConfig['Cognito'], | ||
| oAuthListener: OAuthListener, | ||
| ) { | ||
| if (amplify.listenerTimeout) { | ||
| amplify.clearListenerTimeout(); | ||
| } | ||
| // The setTimeout will only be called as long as an oAuthLister and oauth config | ||
| // are defined. The code executed by the time out will be only fired once. | ||
|
Comment on lines
+111
to
+112
There was a problem hiding this comment. This sounds like it only works with certain preconditions, e.g. assuming With Amplify Gen2, There was a problem hiding this comment. I have to echo this sentiment. When I looked at this PR this was my initial concern as well. Code after the configure call that may have previously executed after the |
||
| amplify.listenerTimeout = setTimeout(() => { | ||
| oAuthListener(config); | ||
| // the listener should only be notified once with a valid oauth config | ||
| amplify.oAuthListener = undefined; | ||
| amplify.clearListenerTimeout(); | ||
| }); | ||
| } | ||
|
|
||
| private clearListenerTimeout() { | ||
| clearTimeout(this.listenerTimeout); | ||
| this.listenerTimeout = undefined; | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this pure would make sense if it were removed completely from the class, but since it is a private member of the class, this seems confusing. For example, it's not really clear to me why
oAuthListeneris passed separately fromthis. TheclearListenerTimeoutis then also tacked onto the instance but it's obfuscated here.