-
Notifications
You must be signed in to change notification settings - Fork 51
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
remove looping gql calls #2620
remove looping gql calls #2620
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces modifications to GraphQL-related files in the Eternum project, primarily focusing on expanding query capabilities and adjusting data retrieval limits. The changes include updating the Changes
Sequence DiagramsequenceDiagram
participant Client
participant GraphQL
participant GameData
Client->>GraphQL: Request Entity Owners
GraphQL->>GameData: Query with expanded limit (10,000)
GameData-->>GraphQL: Return Entity Owner Data
GraphQL-->>Client: Provide Comprehensive Results
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
Failed to generate code suggestions for PR |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
landing/src/hooks/gql/graphql.ts (1)
Line range hint
1-8000
: Architecture advice for query optimizationConsider implementing these optimizations to further reduce GQL calls:
Batch Processing:
- Group related queries using fragments
- Use aliasing for multiple instances of same query type
- Implement DataLoader pattern for N+1 query prevention
Caching Strategy:
- Add cache policies for frequently accessed data
- Implement client-side caching with Apollo Client
- Use partial query caching where appropriate
Query Optimization:
- Use field selection to minimize response payload
- Implement cursor-based pagination for large datasets
- Consider implementing GraphQL subscriptions for real-time updates
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
landing/src/hooks/gql/gql.ts
(2 hunks)landing/src/hooks/gql/graphql.ts
(1 hunks)landing/src/hooks/helpers/useDonkeyArrivals.tsx
(1 hunks)landing/src/hooks/query/entityOwners.tsx
(1 hunks)
🔇 Additional comments (7)
landing/src/hooks/query/entityOwners.tsx (1)
5-5
: Consider verifying performance impact of increasing the limit.
Changing the limit
from 200
to 10000
may impact the performance of the query or the server's response time. Ensure that the query results remain manageable and that any UI interactions are still performant when large datasets are returned.
Run the following script to identify potential usage bottlenecks:
✅ Verification successful
Let me gather more information about the usage of this query and its potential impact.
Let me try a different approach to find the usage of this query.
Let me check how this query is used in the donkey arrivals hook to understand the data handling.
The increased limit appears to be safe in this context
The implementation shows that the query is used efficiently:
- Each realm ID is queried individually rather than in bulk (
realmEntityIds.map
) - Results are cached with a reasonable stale time of 5 minutes
- The data is processed efficiently through memoization
- The query is only used to track donkey positions and resources at specific coordinates, which is naturally limited by game mechanics
Additionally, other similar queries in the codebase also use high limits (e.g., tokenBalances
with limit 8000), suggesting this is an accepted pattern in the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# We scan for all references to getEternumEntityOwner to see if large responses could degrade performance.
rg -A 5 'getEternumEntityOwner'
Length of output: 5135
Script:
#!/bin/bash
# Check where this query is being used and how the results are handled
rg -A 10 'useGetEternumEntityOwner'
Length of output: 35
Script:
#!/bin/bash
# Look for the generated hook name and any references to the query
rg -l "GetEternumEntityOwnerQuery|getEternumEntityOwner" landing/src/
Length of output: 222
Script:
#!/bin/bash
# Check how the query results are used in donkey arrivals
cat landing/src/hooks/helpers/useDonkeyArrivals.tsx
Length of output: 4559
landing/src/hooks/helpers/useDonkeyArrivals.tsx (1)
37-37
: Switch from periodic polling to stale-time caching.
Replacing refetchInterval
with staleTime
helps prevent repeated polling and reduces server load. This is approved, but ensure it aligns with your desired responsiveness.
landing/src/hooks/gql/gql.ts (2)
20-20
: Added limit=10000 for the getEternumEntityOwner query.
As with the corresponding changes elsewhere, confirm this does not exceed system constraints or cause timeouts.
46-46
: Extended limit=10000 in the exported query definition.
The explicit limit increase ensures consistency across the codebase, but verify upstream or downstream usage to ensure it can handle larger responses.
landing/src/hooks/gql/graphql.ts (3)
Line range hint 1-7419
: LGTM: GraphQL schema types are well-defined
The schema types are comprehensive and properly structured with:
- Clear input/output types for queries
- Proper scalar definitions
- Well-organized enums for sorting and filtering
- Consistent naming conventions
Line range hint 7421-8000
: Consistent query limits across the schema
The other queries in the schema use appropriate limits:
getEternumOwnerRealmIds
: 8000 itemsgetAccountTokens
: 8000 itemsgetERC721Mints
: 8000 itemsgetHyperstructureContributions
: 1000 itemsgetEpochs
: 1000 itemsgetEntityPosition
: 8000 itemsgetEntitiesResources
: 8000 items
These limits are well-balanced for their respective use cases.
7420-7420
: Increased query limit to optimize data fetching
The limit for getEternumEntityOwner
query has been increased from 200 to 10000. This change helps reduce the need for multiple paginated queries.
Let's verify if this change is consistent with other queries in the codebase:
✅ Verification successful
Let me search for specific query limit values in the codebase to verify if the increased limit is consistent.
Query limit values are consistent with the codebase patterns
Based on the search results, I can verify that the increased limit of 10000 for getEternumEntityOwner
query is consistent with the codebase patterns. The codebase shows three main limit tiers for GraphQL queries:
- 1000: Used for smaller datasets like epochs and contributions
- 8000: Used for most entity queries (positions, resources, tokens)
- 10000: Used for entity owner queries that need to handle larger datasets
The increased limit from 200 to 10000 aligns with the existing pattern for entity owner queries, as seen in other similar queries like s0EternumEntityOwnerModels
which also uses a limit of 10000.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other query limits in the codebase
# Look for limit parameters in GraphQL queries
rg -A 1 'limit:' | grep -v '10000' | grep -v '8000' | grep -v '1000'
Length of output: 32887
Script:
#!/bin/bash
# Search for specific limit values in GraphQL queries
rg -A 1 'limit: \d+'
Length of output: 11686
* Fix/donkeys empty balance (#2611) * Merge into main (#2603) * Fix missing arrival when sending lords & donkeys (#2594) * contracts: fix share points check * contracts: fix share points check * Add resource filter for contributions (#2596) * Add resource filter for contributions * Change % for specific resource contribution * Fix missing donkeys in bridge step 2 (#2598) * Fix missing donkeys in bridge step 2 * fix rerenders * Remove filter * Fix registration delay (#2602) * Fix registration delay * styling * Open popup on load * Add burned donkeys back (#2601) --------- Co-authored-by: Credence <[email protected]> Co-authored-by: Loaf <[email protected]> * merge into main (#2605) * Fix missing arrival when sending lords & donkeys (#2594) * contracts: fix share points check * contracts: fix share points check * Add resource filter for contributions (#2596) * Add resource filter for contributions * Change % for specific resource contribution * Fix missing donkeys in bridge step 2 (#2598) * Fix missing donkeys in bridge step 2 * fix rerenders * Remove filter * Fix registration delay (#2602) * Fix registration delay * styling * Open popup on load * Add burned donkeys back (#2601) * im (#2604) --------- Co-authored-by: Credence <[email protected]> Co-authored-by: Loaf <[email protected]> * Fix build (#2610) * filter out donkeys w/ empty balance --------- Co-authored-by: Credence <[email protected]> Co-authored-by: Loaf <[email protected]> * add stats resources bridged on empire (#2618) * add resourcers bridged * change logo * remove looping gql calls (#2620) * remove looping gql calls * batch queries --------- Co-authored-by: Credence <[email protected]> Co-authored-by: Loaf <[email protected]> Co-authored-by: raschel <[email protected]>
* Fix/donkeys empty balance (#2611) * Merge into main (#2603) * Fix missing arrival when sending lords & donkeys (#2594) * contracts: fix share points check * contracts: fix share points check * Add resource filter for contributions (#2596) * Add resource filter for contributions * Change % for specific resource contribution * Fix missing donkeys in bridge step 2 (#2598) * Fix missing donkeys in bridge step 2 * fix rerenders * Remove filter * Fix registration delay (#2602) * Fix registration delay * styling * Open popup on load * Add burned donkeys back (#2601) --------- * merge into main (#2605) * Fix missing arrival when sending lords & donkeys (#2594) * contracts: fix share points check * contracts: fix share points check * Add resource filter for contributions (#2596) * Add resource filter for contributions * Change % for specific resource contribution * Fix missing donkeys in bridge step 2 (#2598) * Fix missing donkeys in bridge step 2 * fix rerenders * Remove filter * Fix registration delay (#2602) * Fix registration delay * styling * Open popup on load * Add burned donkeys back (#2601) * im (#2604) --------- * Fix build (#2610) * filter out donkeys w/ empty balance --------- * add stats resources bridged on empire (#2618) * add resourcers bridged * change logo * remove looping gql calls (#2620) * remove looping gql calls * batch queries --------- Co-authored-by: Loaf <[email protected]> Co-authored-by: Bob <[email protected]> Co-authored-by: Credence <[email protected]>
Summary by CodeRabbit
New Features
Performance