-
Notifications
You must be signed in to change notification settings - Fork 199
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
fix(elements-dev-portal): query invalidation on jwt refresh #2550
Conversation
✅ Deploy Preview for stoplight-elements-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for stoplight-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -8,7 +8,7 @@ import { getBranches } from '../handlers/getBranches'; | |||
export function useGetBranches({ projectId }: { projectId: string }) { | |||
const { platformUrl, platformAuthToken } = React.useContext(PlatformContext); | |||
return useQuery( | |||
[...devPortalCacheKeys.branchesList(projectId), platformUrl, platformAuthToken], |
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.
is there a user id or some other context we can use?
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.
or like if the user is actually logged out or not?
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.
@daniel-white We do have state.auth.isLoggedIn
that we can pass through to the provider. Would that work?
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.
Ok added the isLoggedIn
context here 9db422a
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.
Is that going to be an issue for anonymous users who never log in?
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.
@brendarearden I just tested it and it works as an anonymous user. My understanding is the array being passed in to useQuery
is used as a key and invalidated if any of those values change
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!
Elements Default PR Template
In general, make sure you have: (check the boxes to acknowledge you've followed this template)
CONTRIBUTING.md
Other Available PR Templates:
CONTRIBUTING.md