-
Notifications
You must be signed in to change notification settings - Fork 63
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 data fetching issue on Vercel integration page #1241
Conversation
* Requests should be paused if there is no URL * Loading and data should be updated on failed responses * All loading states should be considered
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -72,7 +73,7 @@ export function useVercelIntegration(): { | |||
|
|||
return { | |||
data: vercelIntegration, | |||
fetching: fetching || isLoadingSavedProjects, | |||
fetching: isLoadingEnvironments || isLoadingAllProjects || isLoadingSavedProjects, |
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.
I should have been considering all loading states, but wasn't before
}: { | ||
url: string | URL | null; | ||
method: string; | ||
pause: boolean; |
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.
Added this feature similar to URQL to prevent the URL from being hit dynamically. The url
check on line 28 should have done this, but it might not have updated for some reason. Leaving in for now.
|
||
setIsLoading(true); | ||
const response = await fetch(url, { | ||
method, | ||
headers: { | ||
Authorization: `Bearer ${sessionToken}`, | ||
}, | ||
}); | ||
if (!response.ok || response.status >= 400) { | ||
setData(null); | ||
setIsLoading(false); |
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.
Errors would previously just should stuck as loading before this change.
Description
Hotfix for data not loading on Vercel integration page.
Motivation
Bug reported after #1236 was merged
Type of change (choose one)
Checklist
Check our Pull Request Guidelines