Skip to content

Conversation

@MananTank
Copy link
Member

@MananTank MananTank commented Jan 16, 2025

PR-Codex overview

This PR focuses on fixing the caching issue in NFT components that caused incorrect metadata display when multiple contracts had the same token ID. It introduces contractAddress as a parameter in various functions to ensure proper data retrieval.

Detailed summary

  • Added contractAddress to the query keys in getQueryKey functions for NFTName, NFTMedia, and NFTDescription.
  • Updated tests in media.test.tsx and name.test.tsx to include contractAddress.
  • Fixed metadata display issues in NFT components.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@vercel
Copy link

vercel bot commented Jan 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 8:57pm
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 8:57pm
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 8:57pm
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 8:57pm

@changeset-bot
Copy link

changeset-bot bot commented Jan 16, 2025

🦋 Changeset detected

Latest commit: 4ffcf30

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
thirdweb Patch
@thirdweb-dev/wagmi-adapter Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@linear
Copy link

linear bot commented Jan 16, 2025

@github-actions github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Jan 16, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MananTank MananTank force-pushed the 01-16-_tool-3096_react_sdk_fix_nft_components_not_including_contract_address_in_querykey branch from 379882b to da9f1a5 Compare January 16, 2025 18:08
@MananTank MananTank marked this pull request as ready for review January 16, 2025 18:08
@MananTank MananTank requested review from a team as code owners January 16, 2025 18:08
@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2025

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 46.66 KB (0%) 934 ms (0%) 3.1 s (-13.16% 🔽) 4 s
thirdweb (cjs) 116.84 KB (0%) 2.4 s (0%) 7.1 s (+23.96% 🔺) 9.4 s
thirdweb (minimal + tree-shaking) 5.59 KB (0%) 112 ms (0%) 113 ms (-67.59% 🔽) 224 ms
thirdweb/chains (tree-shaking) 506 B (0%) 10 ms (0%) 88 ms (-35.27% 🔽) 98 ms
thirdweb/react (minimal + tree-shaking) 19.24 KB (0%) 385 ms (0%) 724 ms (-3.26% 🔽) 1.2 s

@codecov
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.82%. Comparing base (fe3f24e) to head (4ffcf30).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5966   +/-   ##
=======================================
  Coverage   54.81%   54.82%           
=======================================
  Files        1150     1150           
  Lines       61195    61200    +5     
  Branches     5153     5154    +1     
=======================================
+ Hits        33545    33550    +5     
  Misses      26923    26923           
  Partials      727      727           
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from fe3f24e
packages 52.47% <100.00%> (+<0.01%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...dweb/src/react/web/ui/prebuilt/NFT/description.tsx 77.77% <100.00%> (+0.41%) ⬆️
...s/thirdweb/src/react/web/ui/prebuilt/NFT/media.tsx 79.01% <100.00%> (+0.53%) ⬆️
...es/thirdweb/src/react/web/ui/prebuilt/NFT/name.tsx 87.50% <100.00%> (+0.40%) ⬆️

... and 4 files with indirect coverage changes

@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 16, 2025

Merge activity

gregfromstl pushed a commit that referenced this pull request Jan 16, 2025
…ess in queryKey (#5966)

<!-- start pr-codex -->

## PR-Codex overview
This PR focuses on improving the handling of NFT metadata by fixing issues with caching when multiple contracts share the same token ID. It updates various components to include the `contractAddress` in the query keys for better differentiation.

### Detailed summary
- Added `contractAddress` to query keys in `getQueryKey` functions in `name.tsx`, `media.tsx`, and `description.tsx`.
- Updated related tests in `name.test.tsx` and `media.test.tsx` to include `contractAddress`.
- Fixed caching issue for NFT components with identical token IDs across contracts.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
@gregfromstl gregfromstl force-pushed the 01-16-_tool-3096_react_sdk_fix_nft_components_not_including_contract_address_in_querykey branch from b49676e to 7d2aa81 Compare January 16, 2025 19:14
gregfromstl pushed a commit that referenced this pull request Jan 16, 2025
…ess in queryKey (#5966)

<!-- start pr-codex -->

## PR-Codex overview
This PR focuses on improving the handling of NFT metadata in components by ensuring that the `contractAddress` is included in the query keys. This change addresses issues with incorrect caching when rendering multiple contracts with the same token ID.

### Detailed summary
- Added `contractAddress` to the query keys in `getQueryKey` functions for `NFTDescription`, `NFTName`, and `NFTMedia`.
- Updated tests in `media.test.tsx` and `name.test.tsx` to include `contractAddress` when calling `getQueryKey`.
- Fixed metadata display issues when multiple contracts with the same token ID are rendered.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
@gregfromstl gregfromstl force-pushed the 01-16-_tool-3096_react_sdk_fix_nft_components_not_including_contract_address_in_querykey branch from 7d2aa81 to f8f0bcb Compare January 16, 2025 19:24
gregfromstl pushed a commit that referenced this pull request Jan 16, 2025
…ess in queryKey (#5966)

<!-- start pr-codex -->

## PR-Codex overview
This PR focuses on fixing issues with NFT components not displaying the correct metadata when multiple contracts share the same token ID. It improves caching by including `contractAddress` in the query keys for NFT metadata retrieval.

### Detailed summary
- Added `contractAddress` to query keys in `getQueryKey` functions for `NFTName`, `NFTMedia`, and `NFTDescription`.
- Updated related queries in `name.tsx`, `media.tsx`, and `description.tsx` to use `contract.address`.
- Enhanced tests in `name.test.tsx` and `media.test.tsx` to validate the inclusion of `contractAddress` in query keys.
- Updated changelog to document the fix for metadata display issues.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
@gregfromstl gregfromstl force-pushed the 01-16-_tool-3096_react_sdk_fix_nft_components_not_including_contract_address_in_querykey branch from f8f0bcb to 1afab7e Compare January 16, 2025 19:26
@vercel vercel bot temporarily deployed to Preview – thirdweb-www January 16, 2025 20:08 Inactive
@vercel vercel bot temporarily deployed to Preview – wallet-ui January 16, 2025 20:08 Inactive
@vercel vercel bot temporarily deployed to Preview – thirdweb_playground January 16, 2025 20:08 Inactive
…ess in queryKey (#5966)

<!-- start pr-codex -->

## PR-Codex overview
This PR focuses on improving the handling of NFT metadata in the `thirdweb` library by adding `contractAddress` to various query keys, which resolves issues with metadata caching when multiple contracts share the same token ID.

### Detailed summary
- Added `contractAddress` to the query keys in `getQueryKey` functions for `NFTName`, `NFTMedia`, and `NFTDescription`.
- Updated related queries to include `contractAddress`.
- Fixed metadata display issues when rendering multiple contracts with the same token ID.
- Enhanced tests in `name.test.tsx` and `media.test.tsx` to validate the new `contractAddress` parameter.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

packages SDK Involves changes to the thirdweb SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants