thirdweb-dev / js

Best in class web3 SDKs for Browser, Node and Mobile apps
https://thirdweb.com
Apache License 2.0
379 stars 259 forks source link

fix(utils): correct filter condition for recommended wallets #3558

Closed gregfromstl closed 4 days ago

gregfromstl commented 4 days ago

Problem solved

Recommended wallets were being compared for object equality when sorting, which caused confusion when using inline createWallet to set the recommended wallets. This PR updates sorting to compare the recommended wallets by ID (installed providers already work this way)


PR-Codex overview

This PR focuses on fixing the recommended wallet ordering in the sortWallets.ts file.

Detailed summary

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

vercel[bot] commented 4 days ago

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

Name Status Preview Comments Updated (UTC)
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 1, 2024 1:37am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 1, 2024 1:37am
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 1, 2024 1:37am
changeset-bot[bot] commented 4 days ago

🦋 Changeset detected

Latest commit: 54c17893307ea4dac13894cb3e07d82821a6012f

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

graphite-app[bot] commented 4 days ago

Your org requires the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

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

gregfromstl commented 4 days ago

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

Join @gregfromstl and the rest of your teammates on Graphite Graphite

codspeed-hq[bot] commented 4 days ago

CodSpeed Performance Report

Merging #3558 will not alter performance

Comparing fix/recommended-wallet-ordering (54c1789) with main (cbb0bc2)

Summary

✅ 9 untouched benchmarks

github-actions[bot] commented 4 days ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 40.47 KB (0%) 810 ms (0%) 2.5 s (-33.08% 🔽) 3.3 s
thirdweb (cjs) 89.99 KB (0%) 1.8 s (0%) 7.1 s (+11.29% 🔺) 8.9 s
thirdweb (minimal + tree-shaking) 4.79 KB (0%) 96 ms (0%) 433 ms (+177.24% 🔺) 529 ms
thirdweb/chains (tree-shaking) 423 B (0%) 10 ms (0%) 61 ms (-41.34% 🔽) 71 ms
thirdweb/react (minimal + tree-shaking) 13.52 KB (0%) 271 ms (0%) 807 ms (-9.73% 🔽) 1.1 s
codecov[bot] commented 4 days ago

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 62.73%. Comparing base (cbb0bc2) to head (54c1789).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3558 +/- ## ========================================== + Coverage 62.69% 62.73% +0.03% ========================================== Files 898 898 Lines 68570 68570 Branches 3670 3685 +15 ========================================== + Hits 42992 43018 +26 + Misses 24890 24863 -27 - Partials 688 689 +1 ``` | [Flag](https://app.codecov.io/gh/thirdweb-dev/js/pull/3558/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | Coverage Δ | | *Carryforward flag | |---|---|---|---| | [legacy_packages](https://app.codecov.io/gh/thirdweb-dev/js/pull/3558/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | `65.59% <ø> (ø)` | | Carriedforward from [cbb0bc2](https://app.codecov.io/gh/thirdweb-dev/js/commit/cbb0bc2835bcfef62e69c18596063842f305f624?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | | [packages](https://app.codecov.io/gh/thirdweb-dev/js/pull/3558/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | `62.19% <0.00%> (+0.04%)` | :arrow_up: | | *This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) to find out more. | [Files](https://app.codecov.io/gh/thirdweb-dev/js/pull/3558?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | Coverage Δ | | |---|---|---| | [...ckages/thirdweb/src/react/web/utils/sortWallets.ts](https://app.codecov.io/gh/thirdweb-dev/js/pull/3558?src=pr&el=tree&filepath=packages%2Fthirdweb%2Fsrc%2Freact%2Fweb%2Futils%2FsortWallets.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev#diff-cGFja2FnZXMvdGhpcmR3ZWIvc3JjL3JlYWN0L3dlYi91dGlscy9zb3J0V2FsbGV0cy50cw==) | `14.81% <0.00%> (ø)` | | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/thirdweb-dev/js/pull/3558/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev)
graphite-app[bot] commented 4 days ago

Merge activity