Skip to content
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 initial auth/me call, skip query until token is defined #228

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

dauglyon
Copy link
Collaborator

Super minor PR to remove a call that always fails (auth/me before we have loaded the login token)

Copy link
Member

@briehl briehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the syntax here. I get that this skips the query, what does authAPIQuery get set to? null? undefined?

I think this is fine, mostly just curious.

@dauglyon
Copy link
Collaborator Author

dauglyon commented Dec 9, 2024

@briehl The skip optional parameter tells rtk-query not to initiate the request via the useQuery hook until it becomes false. Effectively, fetch doesn't run, authAPIQuery.data remains undefined, and authAPIQuery.isUninitialized remains true for any run of the hook which occurs before skip is false. After skip becomes false, fetch is triggered, authAPIQuery.isUninitialized turns false, and after the request response, authAPIQuery.data is populated and a rerender is triggered. Docs here: https://redux-toolkit.js.org/rtk-query/usage/conditional-fetching

its basically all a workaround for hooks not being allowed to be called conditionally during a render.

@dauglyon
Copy link
Collaborator Author

dauglyon commented Dec 9, 2024

It may be better to use skipToken (another way to do this, slightly better-typed behavior), but so far in europa, we've been using the skip param. Switching is easy if we need to in the future.

@briehl
Copy link
Member

briehl commented Dec 9, 2024

Ok, nice. LGTM if that's been the ongoing pattern through now.

@briehl briehl self-requested a review December 9, 2024 22:18
@dauglyon dauglyon merged commit 77d5620 into main Dec 9, 2024
4 checks passed
@dauglyon dauglyon deleted the bugfix-auth-400 branch December 9, 2024 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants