Memoize handleRedirectPromise #3072
Conversation
| @@ -54,6 +54,9 @@ export abstract class ClientApplication { | |||
| // Callback for subscribing to events | |||
| private eventCallbacks: Map<string, EventCallbackFunction>; | |||
|
|
|||
| // Redirect Response Object | |||
| private redirectResponse: Promise<AuthenticationResult | null> | undefined; | |||
There was a problem hiding this comment.
If the page is re-rendered won't this variable be re-rendered also? Doesn't this only solve handleRedirectPromise being called multiple times in a single page load?
There was a problem hiding this comment.
Yes, that's right. More specifically this solves handleRedirectPromise being called multiple times per pca instance.
There was a problem hiding this comment.
in the PR description it says that it may solve issues when frameworks re-render components, we should update this in case someone gets confused thinking this will solve re-rendered instances.
There was a problem hiding this comment.
For a min, I thought the same. :-D. Also @tnorling I do not see page refresh mentioned (or re-rendering) in the note in the description.
There was a problem hiding this comment.
Page refresh is not a broken scenario. This is only to guard against concurrent calls to handleRedirectPromise throwing errors
…ureAD/microsoft-authentication-library-for-js into memoize-handleRedirectPromise
Multiple concurrent calls to
handleRedirectPromisecan cause race conditions and confusing errors when the first invocation clears temporary cache before the 2nd can read it. The official recommendation up until now has been "dont call handleRedirectPromise more than 1 time per page load" but this can be confusing and easy to overlook when using frameworks (react, angular, etc.) that may re-render components and callhandleRedirectPromiseagain unexpectedly.This PR aims to solve this once and for all by having all invocations of
handleRedirectPromiseon a singlePublicClientApplicationinstance return the same promise and ensure that the hash processing logic is only run once.Note: This does not solve scenarios where your page has multiple instances of
PublicClientApplicationcallinghandleRedirectPromiseor wherePublicClientApplicationis re-instantiated and and callshandleRedirectPromiseagain (your app should still only instantiatePublicClientApplicationonce and should not be re-instantiated as a result of a re-render)