-
Notifications
You must be signed in to change notification settings - Fork 7
[FEAT] Add minSecondsBeforeRefresh + debounce refresh token
#43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
src/server/app-router.ts
Outdated
| return undefined | ||
| } | ||
|
|
||
| export async function getUserAndAccessToken(): Promise<{ user?: UserFromToken; accessToken?: string }> { |
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.
Could you change this type so they are either both set or both unset?
src/server/app-router.ts
Outdated
| accessToken: string | ||
| } | ||
| | { | ||
| user: never |
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.
this should be undefined
src/client/AuthProvider.tsx
Outdated
| // If we were offline or on a different tab, when we return, refetch auth info | ||
| // Some browsers trigger focus more often than we'd like, so we'll debounce a little here as well | ||
| const refreshOnOnlineOrFocus = async function () { | ||
| if (lastRefresh && currentTimeSecs() > lastRefresh + minSecondsBeforeRefresh) { |
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.
We should still refresh in the case where lastRefresh is undefined to be safe
andrew-propelauth
left a comment
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.
lgtm
Done as part of: https://www.notion.so/Client-Library-Updates-11753e303ce68051abf0cf4d570ee98e
Background
The original behavior before these changes was that in an application, on page focus, the
/userinfoendpoint would be called each time. This was unnecessary, and would spam these network calls. Now, a user can pass in a propminSecondsBeforeRefreshthat will only call the endpoint after that time has passed. This logic is the same as in the JS library.As well as this, a new function was added to get the user and access token form the server.
Smoke Tests
/userinfoendpoint only gets called after those seconds are up, and after clicking on the page to focus it.getuserAndAccessTokenfunction returned botht he user and access token as expected.