-
Notifications
You must be signed in to change notification settings - Fork 894
Add note about user.reload() method to clarify use #2825
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
this is good information, i agree, but i'm a little confused why user.reload() is called in the first place (and maybe that's also why the user was confused) - we aren't using the user object anywhere, so why do we need to reload it? @royanger originally wrote this guide, maybe he has some insight |
|
We are using the session claims in Middleware and other places. Once the metadata is mutated via BAPI, the user will need an updated session token with updated claims. This happens one of 3 ways:
Until one of the happens, the Middleware (and anything else that checks metadata/session claims) will have stale data. |
Thanks for explanation @royanger - do you think the callout is good /accurate info? |
ahhh that's right, it forces a token refresh and we're using the token in the app in middleware and other places - got it! |
|
There is a code comment already (
|
imo, i don't think it's quite on the money, i think we can frame it slightly better to emphasize that we call it to force a token refresh, and then explain why we need to force the token refresh |
i agree with part of 2 - update the callout with a variation of the info from the feedback but i say we still keep the code comment. maybe update the code comment to say |
|
note we should probably also update https://clerk.com/docs/reference/javascript/user#reload we have some of that info here: https://clerk.com/docs/guides/sessions/force-token-refresh#user-reload whatever we come up with, I think it'd be good to align the two |
|
Have made some updates Refinements post feedback. Let me know your thoughts @alexisintech @royanger. |
alexisintech
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.
posted a small update, to give the information some context so that the user attributes meaning to the copy - otherwise they might be like "what is this, why am i reading this, why is it important" and skip over it !
🔎 Previews:
What does this solve?
This PR comes after user feedback around the use of the
user.reload()method in this page: https://clerk.com/docs/guides/development/add-onboarding-flow. User feedback can be found here. This PR adds a note to clarify its use and links to the corresponding page with more info.What changed?
Checklist