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 infinite loading on field settings #8938

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Fix infinite loading on field settings #8938

merged 2 commits into from
Dec 6, 2024

Conversation

ijreilly
Copy link
Collaborator

@ijreilly ijreilly commented Dec 6, 2024

We were experiencing infinite loading on field settings pages (creation of new field), due to the fact that the component was being rendered on and on and on.
This was due to useGetCurrentUserQuery calls outside of the update function, causing renders in cascade. We also had an issue with the component being unmounted too often.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR fixes infinite loading issues in field settings pages by optimizing GraphQL queries and state management, particularly around user data fetching and component rendering.

  • Replaced useGetCurrentUserQuery with direct Apollo query in useUpdateOneFieldMetadataItem.ts to prevent cascading rerenders
  • Added loading state check in ObjectMetadataItemsLoadEffect.tsx to prevent premature state updates
  • Simplified field settings save flow in SettingsObjectFieldEdit.tsx by moving navigation before updates
  • Reorganized user query fragments to ensure complete data fetching in a single query
  • Added network-only fetch policy for views query to ensure fresh data

6 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +74 to +75
const { data } = await apolloClient.query({ query: GET_CURRENT_USER });
setCurrentWorkspace(data?.currentUser?.defaultWorkspace);
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: No error handling for failed user query or missing defaultWorkspace. Could cause runtime errors if query fails.

Comment on lines +129 to 136
navigate(`/settings/objects/${objectSlug}`);

await updateOneFieldMetadataItem({
objectMetadataId: objectMetadataItem.id,
fieldMetadataIdToUpdate: fieldMetadataItem.id,
updatePayload: formattedInput,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Navigation happens before the async update operation completes. This could lead to race conditions where the user sees stale data if they quickly navigate back.

@charlesBochet charlesBochet merged commit f36555b into main Dec 6, 2024
19 checks passed
@charlesBochet charlesBochet deleted the fix-rerender branch December 6, 2024 17:46
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