fix: _recoverAndRefresh
code efficiency and tidy up
#715
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.
What kind of change does this PR introduce?
Removes unneeded checks, other code and makes things slightly more efficient.
What is the current behavior?
Within
_recoverAndRefresh()
, we're checking forcurrentSession.expires_at
andcurrentSession.refreshToken
as part of a "hey is the session expired?"if
block.We've also left a
_notifyAllSubscribers()
call forSIGNED_IN
, after a recent PR.What is the new behavior?
currentSession.expires_at
andcurrentSession.refresh_token
are already checked for existence within_isValidSession()
, so they've been removed from their respectiveif
blocks. I can also understand if you'd still rather be explicit, even though that means they're getting checked twice.We can also collapse a child
if
block and move it's remaining check forthis.autoRefreshToken
to the parent.We also remove the
SIGNED_IN
emit event that happens towards the end of this function. In all other cases,_notifyAllSubscribers()
is only called if a_saveSession()
or_removeSession()
call preceded it. The one exception is withinconstructor()
, where an event listener, to emit events, is setup for multi-tab communication. The recent PR, referenced above, removed the_saveSession()
call within_recoverAndRefresh()
, so I'd argue there's no need to emitSIGNED_IN
.A possible anti-argument might be something like, "Emitting the
SIGNED_IN
event is just expressing that we noticed someone is signed in during recover and refresh". However, in the case of an expired session, we just refresh the token - causing aTOKEN_REFRESHED
event - and then move on. There's noSIGNED_IN
event emitted in this case. I'd say we either emitSIGNED_IN
in both cases or not at all, but I lean towards not at all.Additional context
Add any other context or screenshots.