-
-
Notifications
You must be signed in to change notification settings - Fork 366
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
feat: add vanilla apollo client #11441
Conversation
…o client integration
WalkthroughThis pull request updates several components and utilities in the codebase. In the NFT creation flow, it adjusts prop handling and replaces a query resolver with direct Apollo client queries. The drops and items modules see the removal of obsolete fetching methods and a shift from string-based to direct query imports. User statistics now feature improved error handling with Apollo queries. Additionally, prop definitions in a UI message component are relaxed, dependencies are updated in package.json, a new Apollo client plugin is introduced, and various GraphQL queries have been updated or relocated. Minor changes are also made to the end-to-end tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Component/Hook
participant ApolloClient as Apollo Client
participant API as GraphQL API
Component->>ApolloClient: Initiate GraphQL query (e.g., getNftId, fetchSearch, profileStatsByIdRefined)
ApolloClient->>API: Send query with custom endpoint (from context)
API-->>ApolloClient: Return query results
ApolloClient-->>Component: Deliver data response
Poem
🪧 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 polkadot ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Updated dependencies detected. Learn more about Socket for GitHub ↗︎
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Deploying koda-art-prod with
|
Latest commit: |
61ae477
|
Status: | ✅ Deploy successful! |
Preview URL: | https://7a20e70f.kodaart-production.pages.dev |
Branch Preview URL: | https://feat--add-vanilla-apollo-cli.kodaart-production.pages.dev |
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: 1
🧹 Nitpick comments (1)
plugins/apollo.client.ts (1)
19-22
: Consider cache configuration options.The Apollo client is configured with a basic in-memory cache. For better performance, consider adding type policies, field policies, or cache redirects based on your data model. Also, evaluate if cache persistence across sessions would be beneficial for your application.
const client = new ApolloClient({ link: customUri, - cache: new InMemoryCache(), + cache: new InMemoryCache({ + typePolicies: { + // Define type policies for better cache behavior + // Example: + // NFTEntity: { + // keyFields: ['id'], + // } + } + }), })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
components/create/CreateNft.vue
(3 hunks)components/drops/useDrops.ts
(0 hunks)components/items/ItemsGrid/useItemsGrid.ts
(3 hunks)composables/useUserStats.ts
(1 hunks)libs/ui/src/components/NeoMessage/NeoMessage.vue
(1 hunks)package.json
(2 hunks)plugins/apollo.client.ts
(1 hunks)queries/subsquid/general/nftByBlockNumber.graphql
(0 hunks)queries/subsquid/general/nftByBlockNumber.ts
(1 hunks)queries/subsquid/general/nftListWithSearch.ts
(1 hunks)queries/subsquid/general/tokenListWithSearch.ts
(2 hunks)tests/e2e/collection.spec.ts
(1 hunks)
💤 Files with no reviewable changes (2)
- queries/subsquid/general/nftByBlockNumber.graphql
- components/drops/useDrops.ts
✅ Files skipped from review due to trivial changes (1)
- queries/subsquid/general/tokenListWithSearch.ts
🔇 Additional comments (17)
queries/subsquid/general/nftByBlockNumber.ts (1)
1-15
: Well-structured query with appropriate filtering and ordering.The GraphQL query is correctly implemented using the
graphql
function from the client module. It has sensible defaults (limit of 10), proper parameter typing, and returns essential fields. The filter onblockNumber_eq
with descending order bysn
will efficiently retrieve the most recent NFTs at a specific block.package.json (2)
54-54
: Adding Apollo client dependency to support direct GraphQL queries.The addition of
@apollo/client
aligns with the PR's objective of implementing vanilla Apollo client functionality. This is a widely used GraphQL client with good community support.
143-143
:❓ Verification inconclusive
Nuxt framework version update.
Updating Nuxt from 3.13.2 to 3.15.4 brings in the latest bug fixes and features. Make sure to check the release notes for any breaking changes that might affect your application.
🌐 Web query:
What are the breaking changes between Nuxt 3.13.2 and 3.15.4?
💡 Result:
Nuxt.js versions 3.13.2 to 3.15.4 introduce several important changes, with three notable breaking changes developers should be aware of:
🔄 CORS Configuration Changes (v3.15.3)
- Default Security Enforcement: The dev server now blocks cross-origin requests by default due to security fixes (GHSA-4gf7-ff8x-hq99).
- Required Configuration for custom hostnames:
export default defineNuxtConfig({ devServer: { cors: { origin: ['https://custom-origin.com'] } } })This impacts developers using non-standard domains during local development[1][3].
🗂 Directory Structure Changes (v4 Compatibility)
When enabling
compatibilityVersion: 4
:
srcDir
defaults toapp/
instead of project rootserverDir
moves to<rootDir>/server
- Layer/module resolution now prioritizes root directory over
srcDir
Projects using legacy structures (e.g., top-levelpages/
) retain backward compatibility unless explicitly opting into v4 behavior[3].🔧 API Changes
- Dedupe Option: Removed boolean shorthand in
useAsyncData
/useFetch
:- await refresh({ dedupe: true }) + await refresh({ cancel: true })- TypeScript Improvements: Stricter type checking for module options and composables[1][5].
Other Notable Changes
- Vite 6 Integration (v3.15): While designed as non-breaking, plugin authors should verify compatibility[8]
- Performance Improvements: Up to 45% faster build times in v3.15 through CLI optimizations[1]
- Security Patches: Critical fixes for SSR context handling and dev server origin validation[1][8]
Developers should:
1️⃣ Review CORS configurations
2️⃣ Check directory structure when enabling v4 features
3️⃣ Replace booleandedupe
usage
4️⃣ Update dependencies withnpx nuxi upgrade --force
[1][8]Citations:
- 1: https://github.com/nuxt/nuxt/releases
- 2: https://manual.mahara.org/en/23.04/blocks/general.html
- 3: https://nuxt.com/docs/getting-started/upgrade
- 4: https://python-dependency-injector.ets-labs.org/main/changelog.html
- 5: https://nuxt.com/blog/v3-13
- 6: https://helm.sh/docs/topics/version_skew/
- 7: Nuxt version is specified to be 3.15.3, but resolving as 3.13.2 nuxt/nuxt#30763
- 8: https://nuxt.com/blog/v3-15
- 9: https://nuxt.com/blog
- 10: https://docs.semaphoreci.com/getting-started/changelog
Nuxt Framework Version Update & Breaking Changes Verification
The dependency update in
package.json
now sets:"nuxt": "^3.15.4",This upgrade—from Nuxt 3.13.2 to 3.15.4—brings along key bug fixes, performance improvements, and security patches. However, be aware of some breaking changes introduced in this version range:
CORS Configuration Changes (v3.15.3):
The dev server now blocks cross-origin requests by default. If you use custom hostnames during local development, update your configuration, for example:export default defineNuxtConfig({ devServer: { cors: { origin: ['https://custom-origin.com'] } } })Directory Structure Adjustments (v4 Compatibility):
When enablingcompatibilityVersion: 4
, note thatsrcDir
now defaults toapp/
instead of the project root, andserverDir
moves to<rootDir>/server
. Projects relying on legacy structures may need to review these changes.API Changes:
The shorthand boolean for thededupe
option in methods likeuseAsyncData
oruseFetch
has been replaced. Update your code accordingly (e.g., replacededupe: true
with the newcancel: true
option).Other Enhancements:
Vite 6 integration and performance optimizations have been introduced, along with improved TypeScript checking.Please review your current configurations and usages to ensure that these changes do not impact your application adversely. For more details, consult the Nuxt release notes.
tests/e2e/collection.spec.ts (1)
88-88
: Test selector updated to match new UI structure.The change from using a test ID to a title attribute selector is appropriate for the UI changes. Ensure that this title attribute is stable and consistently applied across the application to avoid flaky tests.
libs/ui/src/components/NeoMessage/NeoMessage.vue (1)
82-83
: Improved component flexibility by making props optional.Making the
active
andclosable
props optional with default values enhances the reusability of the component, as consumers can now use the component without explicitly specifying these props.composables/useUserStats.ts (4)
1-1
: Direct query import improves code clarity.Importing the GraphQL query directly makes the code more maintainable by establishing clear dependencies between files.
6-6
: Proper Apollo client integration.Adding the Apollo client from the Nuxt app allows for direct query execution which is more consistent with modern GraphQL patterns.
11-20
: Improved query execution with Apollo client.The switch to using Apollo client directly for queries is more idiomatic and aligns with best practices. The context setup with the endpoint is also properly configured.
22-29
: Enhanced error handling for missing data.The addition of a null check for
holdingsEvents
prevents potential runtime errors when data is missing, which is an important defensive programming practice.queries/subsquid/general/nftListWithSearch.ts (1)
1-62
: Well-structured GraphQL query definition with proper fragment composition.This new query is comprehensive and well-organized:
- It properly handles pagination with
first
andoffset
parameters- Includes appropriate filtering for burned NFTs and empty metadata
- Returns a rich set of NFT data including collection information and floor price
- Incorporates reusable fragments for NFT, NFT details, and base metadata
- Includes the total count via the connection object for pagination purposes
The query structure follows best practices for GraphQL by using fragments to modularize the schema.
components/items/ItemsGrid/useItemsGrid.ts (4)
13-14
: Direct query imports improve type safety and maintainability.Importing the query objects directly instead of using string identifiers enables better type checking and makes dependencies explicit, which improves maintainability.
110-113
: Simplified query reference using imported objects.Using the imported query objects directly instead of string identifiers makes the code more robust and reduces the risk of typos in query names.
189-196
: Apollo client integration for query execution.The implementation correctly uses the Apollo client for executing GraphQL queries, with proper context setup that includes the endpoint.
199-199
: Simplified result handling.The code now directly accesses properties from the query result without needing to use
.value
, making the code cleaner.components/create/CreateNft.vue (3)
30-30
: Improved prop handling for optional imageThis change ensures that when
imagePreview
is falsy (null, etc.), the prop explicitly receivesundefined
instead of a falsy value, which likely helps the child component handle default values more effectively.
317-317
: Import for direct GraphQL queryThis import supports the refactoring from dynamic query path resolution to using Apollo client directly.
582-596
: Improved data fetching with Apollo clientThe refactoring from using dynamic query path resolution to direct Apollo client query looks good. This approach is more explicit and provides better type safety.
However, there's limited error handling for the case where the query might not return expected data. Consider adding error handling:
async function getNftId() { try { const result = await $apolloClient.query({ query: nftByBlockNumber, variables: { limit: 1, blockNumber: mintedBlockNumber.value, }, context: { endpoint: currentChain.value, }, }) return result.data?.nftEntities[0]?.id + } catch (error) { + $consola.error('Failed to fetch NFT by block number:', error) + return null + } }
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: 1
🧹 Nitpick comments (2)
components/create/CreateNft.vue (2)
594-596
: Consider adding type annotations for better code documentation.Adding a return type annotation would improve code readability and type safety.
-async function getNftId() { +async function getNftId(): Promise<string | null> { const result = await $apolloClient.query({ // ...query configuration }) return result.data?.nftEntities[0].id }
609-617
: Add more detailed error logging when NFT retrieval fails.The current implementation decrements the retry counter but doesn't provide specific feedback about why the NFT ID couldn't be retrieved. Enhanced logging would help with debugging.
await delay(DETAIL_TIMEOUT) const nftId = await getNftId() if (nftId) { router.push(`/${urlPrefix.value}/gallery/${nftId}`) } else { + $consola.warn(`Failed to retrieve NFT ID, retrying (${retry.value - 1} attempts left)`) retry.value -= 1 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/create/CreateNft.vue
(3 hunks)components/items/ItemsGrid/useItemsGrid.ts
(3 hunks)composables/useUserStats.ts
(1 hunks)plugins/apollo.client.ts
(1 hunks)queries/subsquid/general/nftByBlockNumber.ts
(1 hunks)queries/subsquid/general/nftListWithSearch.ts
(1 hunks)queries/subsquid/general/tokenListWithSearch.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- queries/subsquid/general/nftByBlockNumber.ts
- queries/subsquid/general/tokenListWithSearch.ts
- plugins/apollo.client.ts
- components/items/ItemsGrid/useItemsGrid.ts
- queries/subsquid/general/nftListWithSearch.ts
- composables/useUserStats.ts
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: shard (10, 10)
- GitHub Check: shard (9, 10)
- GitHub Check: shard (8, 10)
- GitHub Check: shard (7, 10)
- GitHub Check: shard (6, 10)
- GitHub Check: shard (5, 10)
- GitHub Check: shard (4, 10)
- GitHub Check: shard (3, 10)
- GitHub Check: shard (2, 10)
- GitHub Check: shard (1, 10)
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (2)
components/create/CreateNft.vue (2)
30-30
: Image prop handling improved.Setting the image prop to either the imagePreview value or explicitly undefined ensures proper prop handling in the child component when no image is available.
317-317
: Direct import of GraphQL query replaces string-based path resolution.This change is part of the Apollo client refactoring, providing better type safety and IDE support compared to the previous approach of resolving query paths.
|
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 (4)
components/create/CreateNft.vue (4)
509-511
: Consider consolidating transaction hooks.You're creating two separate transaction hooks: one at the component level and another specifically for listing. Consider consolidating these to reduce duplication and improve maintainability.
-const { - transaction: listTransaction, - status: listStatus, -} = useTransaction() +const { + transaction, + status, + isLoading, + blockNumber, + isError, + execute: listTransaction, + listStatus, +} = useTransaction({ multiple: true })Note: This suggestion assumes that the
useTransaction
hook can be modified to support multiple transaction tracking. If that's not currently possible, it would require refactoring the hook itself.
532-542
: Add more specific error handling in listNft.The current error handling in
listNft
only logs the error. Consider adding more specific error handling based on the error type.const listNft = async () => { try { await listTransaction(listAction.value, currentChain.value) transactionStatus.value = 'checkListed' } catch (error) { - warningMessage(`${error}`) - $consola.error(error) + $consola.error('Failed to list NFT:', error) + + if (error.message?.includes('insufficient balance')) { + warningMessage($i18n.t('errors.insufficient_balance')) + } else if (error.message?.includes('rejected')) { + warningMessage($i18n.t('errors.transaction_rejected')) + } else { + warningMessage(`${error}`) + } + + transactionStatus.value = 'idle' } }
604-626
: Consider adding a loading state while redirecting.The function that redirects users after NFT creation has retry logic but doesn't clearly communicate the loading state to the user beyond the initial info message.
watchEffect(async () => { if ( mintedBlockNumber.value && retry.value && transactionStatus.value === 'done' ) { + const loadingMessage = infoMessage( $i18n.t('mint.nft.redirect', [DETAIL_TIMEOUT / 1000]), { duration: DETAIL_TIMEOUT }, ) await delay(DETAIL_TIMEOUT) const nftId = await getNftId() if (nftId) { + loadingMessage.close() router.push(`/${urlPrefix.value}/gallery/${nftId}`) } else { retry.value -= 1 + if (retry.value > 0) { + infoMessage($i18n.t('mint.nft.retrying', [retry.value]), { duration: 3000 }) + } else { + errorMessage($i18n.t('mint.nft.redirect_failed')) + } } } })
412-412
:❓ Verification inconclusive
Consider addressing the TODO comment.
This TODO comment suggests that the current implementation might need improvement. Consider evaluating and implementing a better solution for determining the token type.
🏁 Script executed:
#!/bin/bash # Find other occurrences of similar patterns to understand how tokenType should be properly determined rg -A 3 -B 3 "tokenType = computed\(\(\) =>" --type vue --type tsLength of output: 95
Action: Revisit token type determination as indicated by the TODO comment
It appears that the current implementation for computing the token type might benefit from refinement. Our initial search for similar patterns using the command
rg -A 3 -B 3 "tokenType = computed\(\(\) =>" --type vue --type ts
failed due to the unrecognized Vue file type. To continue the verification, please run the updated shell script below, which adds the appropriate file type definitions:
#!/bin/bash # Re-run the search for 'tokenType = computed' across .vue and .ts/.tsx files rg --type-add "vue:*.vue" --type-add "ts:*.ts,*.tsx" -A 3 -B 3 "tokenType = computed"Once you've confirmed how similar computed properties are implemented elsewhere in the codebase, consider refining the token type logic in
components/create/CreateNft.vue
around line 412 accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
components/create/CreateNft.vue
(2 hunks)package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (4)
package.json (1)
54-54
:✅ Verification successful
Appropriate addition of Apollo Client dependency.
Adding the Apollo Client package aligns well with the PR's objective of integrating vanilla Apollo Client functionality. This modern GraphQL client provides improved type safety, caching, and state management capabilities.
🌐 Web query:
What are the latest stable versions of @apollo/client?
💡 Result:
The latest stable version of
@apollo/client
is v3.13.1 [9], released 16 days ago (as of March 3, 2025). This version continues the Apollo Client 3.x line, which remains the actively maintained major version for client-side GraphQL operations. Here are key details:Recent Updates and Features (v3.13.x)
useSuspenseFragment
Hook: Introduced in v3.13.0 as a drop-in replacement foruseFragment
in React Suspense-enabled apps, improving fragment handling in asynchronous contexts [1][9].- TypeScript Improvements: Enhanced type safety for
updateQuery
andsubscribeToMore
, including support for early exits viaundefined
returns and more accurate variable typing [1].- Deprecations:
onCompleted
andonError
callbacks inuseQuery
/useLazyQuery
(due to lifecycle issues) [1].ignoreResults
option inuseMutation
(replaced by directclient.mutate
usage) [1].Stability and Migration Context
- Apollo Client 3.x is the recommended major version, with no current plans for a major version 4 [4][8].
- Recent patches (e.g., v3.12.9, v3.13.1) focus on bug fixes, multipart response handling, and type-system refinements [1][9].
- Older versions like Apollo Client 2.x and 1.x are deprecated or end-of-life [2][6].
For most projects, upgrading to v3.13.1 is advised to leverage performance optimizations, React 19 compatibility, and improved type safety [7][9]. Always review the changelog for breaking changes during upgrades [10].
Citations:
- 1: https://github.com/apollographql/apollo-client/releases
- 2: https://www.apollographql.com/docs/apollo-server/previous-versions
- 3: https://www.apollographql.com/docs/react/v2
- 4: https://www.apollographql.com/docs/react/migrating/apollo-client-3-migration
- 5: https://community.apollographql.com/t/upgrade-from-3-6-9-to-3-11-10/7944
- 6: https://www.npmjs.com/package/apollo-client
- 7: https://community.apollographql.com/t/upgrade-from-3-11-to-3-12/8015
- 8: https://www.apollographql.com/docs/react
- 9: https://www.npmjs.com/package/@apollo/client
- 10: https://github.com/apollographql/apollo-client/blob/main/CHANGELOG.md
Verified: @apollo/client Dependency Version is Up-to-Date
The package.json now correctly includes
"@apollo/client": "^3.13.1"
, which is confirmed as the latest stable version. This addition aligns perfectly with the PR's goals by integrating Apollo Client's vanilla functionality, benefitting from its enhanced type safety, caching, and state management features.components/create/CreateNft.vue (3)
30-30
: Good defensive programming with empty string fallback.The change to pass an empty string when imagePreview is falsy prevents potential rendering issues in the child component.
315-315
: Import change aligns with Apollo client integration.Importing the GraphQL query directly rather than using a query path resolver is consistent with modern Apollo client practices and improves type safety.
578-603
: Improved error handling in Apollo query implementation.The refactored
getNftId
function now properly handles edge cases with robust error checking and logging. The try-catch block and empty results validation are excellent additions.Note that this implementation addresses the concerns raised in the previous review comment about error handling and validation of query results.
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
Chores
Tests