Skip to content

Conversation

@gregfromstl
Copy link
Contributor

@gregfromstl gregfromstl commented Oct 18, 2024

Problem solved

Short description of the bug fixed or feature added


PR-Codex overview

This PR focuses on fixing revalidation issues with siwe authentication in the ConnectEmbed component by updating the mutation keys to include the activeAccount address.

Detailed summary

  • Updated mutationKey for login to include activeAccount?.address.
  • Updated mutationKey for logout to include activeAccount?.address.
  • Added gcTime, placeholderData, and refetchOnWindowFocus options in the hook configuration.

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

@changeset-bot
Copy link

changeset-bot bot commented Oct 18, 2024

🦋 Changeset detected

Latest commit: 67a363f

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

This PR includes changesets to release 1 package
Name Type
thirdweb 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

@vercel
Copy link

vercel bot commented Oct 18, 2024

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 Oct 18, 2024 4:26pm
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 18, 2024 4:26pm
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 18, 2024 4:26pm
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 18, 2024 4:26pm

@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 42.81 KB (0%) 857 ms (0%) 716 ms (-3.61% 🔽) 1.6 s
thirdweb (cjs) 102.66 KB (0%) 2.1 s (0%) 1.5 s (-15.1% 🔽) 3.5 s
thirdweb (minimal + tree-shaking) 4.84 KB (0%) 97 ms (0%) 25 ms (-58.77% 🔽) 122 ms
thirdweb/chains (tree-shaking) 498 B (0%) 10 ms (0%) 29 ms (+177.69% 🔺) 39 ms
thirdweb/react (minimal + tree-shaking) 17.36 KB (0%) 348 ms (0%) 351 ms (+186.69% 🔺) 699 ms


const queryClient = useQueryClient();

// We need to reset the react query cache on mount for components like ConnectEmbed that are unmount and remount depending on the auth / connection state
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect useQuery to already refetch on mount? That is the standard behavior.

Copy link
Contributor Author

gregfromstl commented Oct 18, 2024

Merge activity

  • Oct 18, 12:15 PM EDT: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Oct 18, 12:23 PM EDT: A user added this pull request to the Graphite merge queue.
  • Oct 18, 12:26 PM EDT: A user merged this pull request with the Graphite merge queue.

@codecov
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 45.29%. Comparing base (1cd11a8) to head (67a363f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../thirdweb/src/react/core/hooks/auth/useSiweAuth.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5080      +/-   ##
==========================================
- Coverage   45.29%   45.29%   -0.01%     
==========================================
  Files        1059     1059              
  Lines       54728    54729       +1     
  Branches     3954     3957       +3     
==========================================
  Hits        24791    24791              
- Misses      29246    29247       +1     
  Partials      691      691              
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from 1cd11a8
packages 40.23% <33.33%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
.../thirdweb/src/react/core/hooks/auth/useSiweAuth.ts 37.64% <33.33%> (-1.64%) ⬇️

... and 1 file with indirect coverage changes

## Problem solved

Short description of the bug fixed or feature added

<!-- start pr-codex -->

---

## PR-Codex overview
This PR focuses on fixing revalidation issues related to `siwe` authentication in the `ConnectEmbed` component by updating mutation keys to include the `activeAccount` address.

### Detailed summary
- Updated `mutationKey` for the `login` mutation to include `activeAccount?.address`.
- Updated `mutationKey` for the `logout` mutation to include `activeAccount?.address`.
- Added `gcTime`, `placeholderData`, and `refetchOnWindowFocus` options in the hook configuration.

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

<!-- end pr-codex -->
@gregfromstl gregfromstl force-pushed the fix/siwe-auth-revalidation branch from a192d30 to 67a363f Compare October 18, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Adds the pull request to Graphite's merge queue. packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants