♻️ Remove explicit invalidate calls #1445
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe pull request introduces several modifications across various components in the application, primarily focusing on the removal of query invalidation logic associated with TRPC mutations. This change simplifies the handling of mutation success events across components, including account deletion, registration, login, and poll management. The overall structure of the components remains intact, with no alterations to user interface elements or form handling. The changes reflect a trend towards a more streamlined state management approach, reducing unnecessary network requests while maintaining core functionalities. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/src/components/poll/notifications-toggle.tsx (1)
Line range hint
38-77: Well-implemented optimistic updates for mutations.The mutation handlers implement optimistic updates correctly using
setData, which is the recommended approach. This aligns well with the PR's objective of removing explicit invalidate calls, as the optimistic updates maintain UI consistency without requiring query invalidation.The current implementation is a good example of proper state management in TRPC:
- Uses optimistic updates to maintain UI responsiveness
- Properly handles the cache updates without relying on invalidation
- Maintains data consistency across components
apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx (1)
Line range hint
215-251: Consider improving mutation success handling.While removing explicit invalidation is correct, consider these improvements for better UX:
- Add loading state handling to prevent dialog from closing before mutation completes
- Add error handling to show feedback on mutation failure
const scheduleEvent = trpc.polls.book.useMutation(); +const scheduleEvent = trpc.polls.book.useMutation({ + onError: (error) => { + // Show error toast/notification + }, +}); // In onSubmit: -scheduleEvent.mutate({ - pollId: poll.id, - optionId: data.selectedOptionId, - notify: data.notify, -}); -props.onOpenChange?.(false); +scheduleEvent.mutate( + { + pollId: poll.id, + optionId: data.selectedOptionId, + notify: data.notify, + }, + { + onSuccess: () => { + props.onOpenChange?.(false); + }, + }, +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
apps/web/src/app/[locale]/(admin)/settings/profile/delete-account-dialog.tsx(0 hunks)apps/web/src/app/[locale]/(auth)/register/register-page.tsx(0 hunks)apps/web/src/app/[locale]/auth/login/login-page.tsx(0 hunks)apps/web/src/app/[locale]/poll/[urlId]/duplicate-dialog.tsx(0 hunks)apps/web/src/components/auth/auth-forms.tsx(1 hunks)apps/web/src/components/create-poll.tsx(0 hunks)apps/web/src/components/discussion/discussion.tsx(0 hunks)apps/web/src/components/participant-dropdown.tsx(0 hunks)apps/web/src/components/poll/manage-poll.tsx(0 hunks)apps/web/src/components/poll/manage-poll/delete-poll-dialog.tsx(0 hunks)apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx(1 hunks)apps/web/src/components/poll/mutations.ts(0 hunks)apps/web/src/components/poll/notifications-toggle.tsx(1 hunks)
💤 Files with no reviewable changes (10)
- apps/web/src/app/[locale]/(admin)/settings/profile/delete-account-dialog.tsx
- apps/web/src/app/[locale]/(auth)/register/register-page.tsx
- apps/web/src/app/[locale]/auth/login/login-page.tsx
- apps/web/src/app/[locale]/poll/[urlId]/duplicate-dialog.tsx
- apps/web/src/components/create-poll.tsx
- apps/web/src/components/discussion/discussion.tsx
- apps/web/src/components/participant-dropdown.tsx
- apps/web/src/components/poll/manage-poll.tsx
- apps/web/src/components/poll/manage-poll/delete-poll-dialog.tsx
- apps/web/src/components/poll/mutations.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/components/auth/auth-forms.tsx
🔇 Additional comments (3)
apps/web/src/components/poll/notifications-toggle.tsx (1)
127-131: LGTM! Clean formatting of translation default values.
The formatting changes to the translation default values improve code consistency.
apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx (2)
215-215: LGTM! Removal of explicit invalidation is correct.
The removal of explicit query invalidation aligns with the PR objective, as mutations automatically invalidate related queries.
215-215: Verify automatic invalidation coverage.
Let's verify that automatic invalidation properly updates all dependent queries.
✅ Verification successful
Let me gather more information with corrected file type flags.
Let me try with a different approach using more basic commands.
Let me check the router implementation and invalidation settings.
Let me check one more aspect focusing on the mutation implementation and its usage.
Based on the code analysis, I can now generate the final response:
Automatic invalidation is properly handled - no explicit invalidation needed
The polls.book mutation is used in a self-contained flow within the finalize-poll-dialog component. The mutation updates the poll's status to "finalized" and creates an event, which triggers tRPC's automatic cache invalidation. Related queries (polls.get, polls.getWatchers) will be automatically invalidated due to tRPC's built-in cache management.
- The mutation changes the poll's status which automatically invalidates
polls.getqueries - The component properly handles the success case by closing the dialog
- No other components directly depend on this mutation's result
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for components and queries that might be affected by this mutation
# Find components that might be watching the poll data
echo "Looking for components that depend on poll data..."
rg -l "polls\." --type tsx
# Find related query definitions
echo "Looking for related query definitions..."
rg "polls\.book" --type ts
Length of output: 408
Script:
#!/bin/bash
# Search for components using polls-related queries with correct file extension
echo "Components using polls queries:"
rg -l "polls\." --type typescript --type typescriptreact
# Look for query invalidation settings in the router
echo "\nRouter invalidation settings:"
rg -B5 -A5 "polls.book" --type typescript
# Check for other mutations that might affect poll data
echo "\nOther poll-related mutations:"
rg "polls\." --type typescript | grep -i "useMutation"
# Look for query definitions that might be affected
echo "\nPoll-related queries:"
rg "polls\." --type typescript | grep -i "useQuery"
Length of output: 623
Script:
#!/bin/bash
# Find all TypeScript/React files first
echo "Finding all TypeScript and React files:"
fd "\.(ts|tsx)$"
# Then use these files to search for polls-related queries
echo -e "\nSearching for poll-related queries and mutations:"
fd "\.(ts|tsx)$" --exec rg -l "polls\."
# Look specifically for the book mutation and surrounding context
echo -e "\nSearching for book mutation context:"
fd "\.(ts|tsx)$" --exec rg -B5 -A5 "polls\.book" {}
# Search for tRPC router definitions
echo -e "\nSearching for tRPC router definitions:"
fd "router\.(ts|tsx)$" --exec rg -l "polls"
Length of output: 19184
Script:
#!/bin/bash
# Check the polls router implementation for invalidation settings
echo "Checking polls router implementation:"
rg -B10 -A10 "book.*=.*procedure" apps/web/src/trpc/routers/polls.ts
# Look for query definitions that might be affected by this mutation
echo -e "\nChecking query definitions in polls router:"
rg "useQuery.*polls\." apps/web/src/components/poll/
# Check for any invalidation calls in the finalize dialog
echo -e "\nChecking finalize dialog implementation:"
rg -A10 "polls\.book\.useMutation" apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx
Length of output: 842
Script:
#!/bin/bash
# Check the entire finalize dialog implementation for mutation usage
echo "Checking complete finalize dialog implementation:"
cat apps/web/src/components/poll/manage-poll/finalize-poll-dialog.tsx
# Check the polls router for book procedure implementation
echo -e "\nChecking polls router for book procedure:"
cat apps/web/src/trpc/routers/polls.ts
# Look for any related queries that might need invalidation
echo -e "\nChecking for related queries:"
rg "polls\.(get|by|find)" apps/web/src/components/poll/
Length of output: 33622
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/web/src/trpc/client.ts (1)
10-10: Consider the implications of using unstable API featuresThe
unstable_prefix indicates this is an experimental API that may change in future versions. Consider:
- Documenting this decision
- Adding a version pin for tRPC
- Creating a migration plan for when this API stabilizes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/src/components/discussion/discussion.tsx(0 hunks)apps/web/src/trpc/client.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/components/discussion/discussion.tsx
🔇 Additional comments (1)
apps/web/src/trpc/client.ts (1)
13-14: 🛠️ Refactor suggestion
Verify the performance impact of blanket query invalidation
Invalidating all queries after every mutation could be inefficient if the application has many queries that are unrelated to the mutation.
Let's analyze the query usage in the codebase:
Consider implementing more selective invalidation based on the mutation type:
async onSuccess(opts) {
await opts.originalFn();
- await opts.queryClient.invalidateQueries();
+ // Extract mutation path from opts and invalidate related queries only
+ const mutationPath = opts.mutation.options.mutationKey[0];
+ await opts.queryClient.invalidateQueries({ queryKey: [mutationPath] });
},✅ Verification successful
Blanket query invalidation is acceptable in this codebase
The codebase has minimal query usage with only one query for authentication providers that is explicitly set with Infinity for both cacheTime and staleTime. This means the blanket invalidation won't cause unnecessary re-fetches since:
- The providers query won't be affected due to its infinite stale time
- There's only one mutation related to authentication
- The invalidation appears in auth-related contexts where a full cache clear is appropriate
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Analyze query usage to assess the impact of blanket invalidation
# Find all tRPC query definitions
echo "=== Query Definitions ==="
ast-grep --pattern 'useQuery($$$)'
# Find all tRPC mutation definitions
echo "=== Mutation Definitions ==="
ast-grep --pattern 'useMutation($$$)'
# Look for existing selective invalidation patterns
echo "=== Existing Invalidation Patterns ==="
rg -A 2 'invalidateQueries\('
Length of output: 2764
We don't need to invalidate queries explicitly since we invalidate all queries on any mutation
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
unstable_overridesproperty for enhanced mutation handling in the TRPC client.