fix: asyncdata already mounted part 3 #11491
Conversation
WalkthroughThis pull request refactors several components and utility modules by updating data fetching methods, type annotations, and control flows. Changes include replacing GraphQL hooks with direct Apollo Client queries, introducing new queries and fragments, and removing deprecated functions. The carousel and activity components have been updated for better data handling and conditional rendering based on the existence of relevant items. Additionally, type adjustments and modifications to exported functions improve clarity and maintainability throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant CarouselComp as CarouselTypeGenerative.vue
participant UseEvents as useEvents (Carousel Utils)
participant DataMerge as Computed Data
participant CarouselIdx as CarouselIndex Component
CarouselComp->>CarouselComp: Receive collectionIds prop
CarouselComp->>UseEvents: Call useEvents(chain, type, limit, collectionIds)
UseEvents->>DataMerge: Fetch ahpEventsNewestList & ahpEventsLatestSales
DataMerge->>DataMerge: Merge, deduplicate (via unionBy), and sort events
CarouselComp->>CarouselIdx: Conditionally render CarouselIndex if data array is non-empty
sequenceDiagram
participant Activity as Activity.vue
participant Apollo as Apollo Client
participant Query as allEventsByProfile Query
participant SortBy as Lodash sortBy
Activity->>Apollo: Execute allEventsByProfile query onMounted
Apollo->>Activity: Return events data
Activity->>SortBy: Sort events based on timestamp
Activity->>Activity: Filter events by string type (e.g., 'buy', 'sale')
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (18)
💤 Files with no reviewable changes (2)
🪧 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 (
|
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
preschian
requested review from
vikiival and
Jarsen136
and removed request for
a team
Deploying koda-art-prod with
|
| Latest commit: |
7ecb084
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://29f6342c.kodaart-production.pages.dev |
| Branch Preview URL: | https://refactor--asyncdata-already.kodaart-production.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
components/landing/LandingPage.vue (1)
21-24: Ensure defensive checks fordropsAhpbefore accessing.length.
IfgetDropsfails or returnsundefined, this might throw an error. Consider making a null-safe check or guaranteeingdropsAhpis always an array.You might, for example:
-<LazyCarouselTypeGenerative - v-if="dropsAhp.length" - :collection-ids="dropsAhp" /> +<LazyCarouselTypeGenerative + v-if="dropsAhp?.length" + :collection-ids="dropsAhp" />components/carousel/utils/useCarouselEvents.ts (1)
80-84: Potential race condition in data fetchingThe
whereobject used in the fetch call depends on reactive references (excludeNftsandexcludeCollections), but the fetch is called only once when the function is first executed. If these values change after the initial fetch, the query won't be re-executed with the updated values.Consider moving the fetch call inside a watchEffect to re-fetch when dependencies change:
- fetchLatestEvents(chain, type, where).then((result) => { - data.value = result.data - }) + watchEffect(async () => { + const currentWhere = createEventQuery( + type, + excludeNfts, + excludeCollections, + collectionIds, + ) + const result = await fetchLatestEvents(chain, type, currentWhere) + data.value = result.data + })components/collection/utils/useCollectionDetails.ts (2)
26-36: Consider adding error handling to fetchStatsThe
fetchStatsfunction looks good but lacks error handling, which could lead to silent failures if the API request fails.const fetchStats = () => { $apolloClient.query({ query: collectionStatsById, variables: variables.value, context: { endpoint: urlPrefix.value, }, - }).then((res) => { - data.value = res.data - }) + }).then((res) => { + data.value = res.data + }).catch((error) => { + console.error('Error fetching collection stats:', error) + }) }
96-114: Inconsistent pattern with useCollectionSoldDataUnlike
useCollectionDetails, theuseCollectionSoldDatafunction executes the query immediately without providing a dedicated function for re-fetching, and lacks error handling.Consider refactoring to match the pattern used in
useCollectionDetails:export function useCollectionSoldData({ address, collectionId }) { const data = ref<ResultOf<typeof nftListSoldByCollection>>() const { $apolloClient } = useNuxtApp() const { urlPrefix } = usePrefix() - $apolloClient.query({ - query: nftListSoldByCollection, - variables: { - account: address, - limit: 3, - orderBy: 'price_DESC', - collectionId, - }, - context: { - endpoint: urlPrefix.value, - }, - }).then((res) => { - data.value = res.data - }) + const fetchSoldData = () => { + $apolloClient.query({ + query: nftListSoldByCollection, + variables: { + account: address, + limit: 3, + orderBy: 'price_DESC', + collectionId, + }, + context: { + endpoint: urlPrefix.value, + }, + }).then((res) => { + data.value = res.data + }).catch((error) => { + console.error('Error fetching sold NFT data:', error) + }) + } + + fetchSoldData() - return { nftEntities: computed(() => data.value?.nftEntities?.length ? data.value.nftEntities : []) } + return { + nftEntities: computed(() => data.value?.nftEntities?.length ? data.value.nftEntities : []), + refetch: fetchSoldData + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
components/carousel/CarouselTypeGenerative.vue(1 hunks)components/carousel/utils/useCarouselEvents.ts(5 hunks)components/collection/utils/useCollectionDetails.ts(5 hunks)components/identity/utils/useIdentity.ts(0 hunks)components/identity/utils/useIdentityStats.ts(2 hunks)components/landing/LandingPage.vue(3 hunks)components/profile/ProfileActivitySummery.vue(1 hunks)components/profile/activityTab/Activity.vue(2 hunks)composables/useSearchNfts.ts(2 hunks)plugins/apollo.client.ts(1 hunks)queries/fragments/typed/collectionMeta.ts(1 hunks)queries/subsquid/general/allEventsByProfile.ts(2 hunks)queries/subsquid/general/collectionStatsById.ts(1 hunks)queries/subsquid/general/nftListSoldByCollection.ts(2 hunks)queries/subsquid/general/userStatsByAccount.ts(2 hunks)types/index.ts(1 hunks)utils/math.ts(1 hunks)utils/sorting.ts(0 hunks)
💤 Files with no reviewable changes (2)
- components/identity/utils/useIdentity.ts
- utils/sorting.ts
🧰 Additional context used
🧬 Code Definitions (1)
queries/subsquid/general/nftListSoldByCollection.ts (1)
queries/fragments/typed/collectionMeta.ts (1) (1)
collectionMetaFragment(3-9)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: shard (1, 10)
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (26)
queries/subsquid/general/userStatsByAccount.ts (1)
1-3: Improved query export structure using graphql client wrapperThe refactoring correctly wraps the GraphQL query with the
graphqlfunction imported from the client module, providing better type safety and a more consistent approach to query definitions across the codebase. This aligns with the PR objective of refactoring for better maintainability.Also applies to: 41-41
queries/subsquid/general/allEventsByProfile.ts (1)
1-4: Well-structured query export with proper formattingThe implementation correctly wraps the GraphQL query with the
graphqlfunction from the client module, maintaining proper formatting with template literals. This standardized approach to query definitions enhances maintainability and type safety across the codebase.Also applies to: 32-33
queries/fragments/typed/collectionMeta.ts (1)
1-9: Good implementation of reusable GraphQL fragmentThe introduction of this GraphQL fragment is a positive addition that promotes code reuse and modularity. By extracting the collection metadata into a reusable fragment, the code reduces duplication and improves maintainability when this data needs to be used across multiple queries.
plugins/apollo.client.ts (1)
8-8: Good addition of fallback endpoint valueThis change adds a necessary fallback mechanism that prevents potential runtime errors when the endpoint is not provided in the context. By defaulting to 'ahp' when endpoint is falsy, the code ensures that the Apollo client always has a valid GraphQL endpoint to connect to.
components/profile/ProfileActivitySummery.vue (1)
76-76: Ensure consistent data type handling.Converting
listedValueto a string is appropriate since theCommonTokenMoneycomponent likely expects string input for proper rendering. This change improves type safety and prevents potential rendering issues with bigint values.queries/subsquid/general/collectionStatsById.ts (1)
1-26: Well-structured GraphQL query for collection statistics.The query is well organized and fetches the necessary data efficiently. It properly separates different types of NFTs (base, listed, sales) which allows for clear data processing on the client side.
components/landing/LandingPage.vue (1)
45-45: Import looks appropriate.
No issues detected with this import usage.queries/subsquid/general/nftListSoldByCollection.ts (3)
1-4: Imports are clear and modular.
Transitioning to explicit ES imports is a recommended practice for clarity.
6-6: Exporting a default GraphQL query object is a clean approach.
No concerns here.
32-38: Embedding a fragments array is a structured strategy.
This approach nicely keeps your fragments grouped with the query for improved maintainability.composables/useSearchNfts.ts (3)
1-2: Imports look correct.
No issues with referencingResultOfornftListWithSearch.
39-41: Immediate fetch is straightforward.
This ensures data is loaded quickly, though remember to handle any potential concurrency with overlapping requests if triggered multiple times.
45-45: Refetch alias is consistent.
No further comments; approach looks good.components/carousel/CarouselTypeGenerative.vue (3)
4-4: Good conditional rendering improvement!Adding this v-if condition prevents rendering the carousel when there's no data, which is a great UI improvement.
15-16: Improved data handling with proper collection filteringThis refactoring improves the carousel component by:
- Using the utility functions directly rather than relying on a custom hook
- Properly consolidating data from two different event sources
- Using
unionByto remove duplicates based on 'id'- Ensuring proper sorting for display
The approach is more maintainable and allows for better control over the data flow.
Also applies to: 31-39
19-21: Good component design with propsMaking the component accept
collectionIdsas a prop improves reusability and encapsulation. This allows parent components to control which collections should be displayed.components/profile/activityTab/Activity.vue (1)
63-63: Good migration from enums to string literalsThe refactoring from using
InteractionEnumto string literals makes the code easier to understand and maintain. The mapping object now clearly shows which interaction types correspond to which filter categories.The updated check for 'BUY' interaction in the filter logic is also correct.
Also applies to: 102-106, 109-112
components/carousel/utils/useCarouselEvents.ts (1)
54-54: Good function exports and type safety improvementsMaking
useEventsandsortNftsexported functions improves modularity and reusability across the codebase. The function can now be imported in other components, as seen inCarouselTypeGenerative.vue.Adding proper type annotation to the
dataref withResultOf<typeof latestEvents>improves type safety and developer experience.Also applies to: 81-81, 152-152
components/collection/utils/useCollectionDetails.ts (8)
2-3: Import structure improved with proper type importsThe imports have been updated to include proper typing with
ResultOffrom gql.tada, which provides better type safety for GraphQL queries. The new imports for collection stats and sold NFT list queries support the refactored data fetching approach.Also applies to: 5-9
20-22: Good setup of Apollo client and URL prefixProperly extracting the Apollo client and URL prefix from the Nuxt app to support direct query execution.
23-23: Type safety improvement with ResultOfUsing
ResultOf<typeof collectionStatsById>provides precise typing that reflects the structure of the data returned by the query, which improves type safety and IDE autocompletion.
38-38: Initial data fetchingImmediately calling
fetchStats()ensures data is loaded when the component mounts, which is appropriate for this use case.
51-51: Improved null safety with nullish coalescingGood use of the nullish coalescing operator (
??) to handle potentially undefined values formaxSupplyand item prices, making the code more robust.Also applies to: 56-56
70-70: Reactive data fetching with watchSetting up a watcher to call
fetchStatswhen variables change ensures the data stays in sync with the query parameters.
74-74: Updated refetch methodUpdating the
refetchreference to use the newfetchStatsfunction aligns with the refactored data-fetching strategy.
116-116: Good use of computed property with null checkingThe computed property properly handles cases where
nftEntitiesmight be null, undefined, or empty by returning an empty array as a fallback.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the duplicate items issue on the carousel landing page by refactoring several GraphQL queries and related components while also updating type definitions and query handling. Key changes include the introduction of new GraphQL queries using Apollo Client, modifications to type definitions (e.g. changing blockNumber to allow null), and refactoring of carousel event handling and sorting.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| queries/subsquid/general/collectionStatsById.ts | New query for collection stats |
| composables/useSearchNfts.ts | Replaced useGraphql with Apollo Client query |
| components/collection/utils/useCollectionDetails.ts | Updated query fetching & data processing |
| queries/fragments/typed/collectionMeta.ts | Added new fragment for collection metadata |
| components/identity/utils/useIdentityStats.ts | Updated user stats query with Apollo Client |
| plugins/apollo.client.ts | Added fallback endpoint for GraphQL client |
| queries/subsquid/general/allEventsByProfile.ts | Removed unnecessary __typename fields |
| queries/subsquid/general/userStatsByAccount.ts | Updated user stats query definition |
| queries/subsquid/general/nftListSoldByCollection.ts | Modernized query fragment imports |
| types/index.ts | Updated blockNumber type definition |
| components/carousel/utils/useCarouselEvents.ts | Refactored carousel event fetching and sorting |
| utils/math.ts | Adjusted getVolume function signature |
| utils/sorting.ts | Removed unused sorting function |
| components/carousel/CarouselTypeGenerative.vue | Updated carousel component to use new event fetching |
| components/identity/utils/useIdentity.ts | Removed unused identity sold data helper |
| components/landing/LandingPage.vue | Updated landing page to conditionally display carousel |
| components/profile/ProfileActivitySummery.vue | Converted listedValue to string for display |
| components/profile/activityTab/Activity.vue | Updated profile activities handling with Apollo Client |
Comments suppressed due to low confidence (1)
types/index.ts:127
- Changing the type of blockNumber from 'string | number' to 'string | null' may affect parts of the code that assume a number; please confirm that all references handle a possible null value.
blockNumber: string | null
…state management in useSearchNfts
|
Thank you for your contribution to the Koda - Generative Art Marketplace.
👇 __ Let's make a quick check before the contribution.
PR Type
Context
Summary by CodeRabbit
New Features
Refactor