(5.0) fix: handleLoginRedirect should not subscribe to auth state changes #730
Conversation
aarongranick-okta
commented
May 11, 2021
aarongranick-okta
commented
- updateAuthState now returns a promise that can be awaited
Codecov Report
@@ Coverage Diff @@
## master #730 +/- ##
==========================================
+ Coverage 81.05% 81.15% +0.09%
==========================================
Files 103 103
Lines 2967 2972 +5
Branches 601 602 +1
==========================================
+ Hits 2405 2412 +7
+ Misses 562 560 -2
Continue to review full report at Codecov.
|
| @@ -67,7 +67,7 @@ export class AuthStateManager { | |||
| return this._authState; | |||
| } | |||
|
|
|||
| updateAuthState(): void { | |||
| async updateAuthState(): Promise<AuthState> { | |||
There was a problem hiding this comment.
Should here be considered as a breaking change? Technically, we are exposing authStateManager from authClient (https://github.com/okta/okta-auth-js/blob/master/lib/OktaAuth.ts#L260)
There was a problem hiding this comment.
probably not a breaking change, as app code won't change because of this signature change.
| @@ -92,12 +92,22 @@ export class AuthStateManager { | |||
| devMode && log('emitted'); | |||
| }; | |||
|
|
|||
| const finalPromise = (origPromise) => { | |||
There was a problem hiding this comment.
How about making the finalPromise returns a function instead of a promise? In this way, I think we can resolve the concern about the promise may become stale.
OKTA-391495 <<<Jenkins Check-In of Tested SHA: 2e329e8 for [email protected]>>> Artifact: okta-auth-js Files changed count: 6 PR Link: "#730"
