feat: Wallet connection improvements #2632

MananTank · 2024-04-02T15:25:42Z

PR-Codex overview

This PR improves wallet connection handling by changing the accountsChanged event to accountChanged and emitting a new Account object.

Detailed summary

Renamed accountsChanged event to accountChanged event
Emit new Account object instead of creating it in the connection manager
Enhanced WalletConnect connection handling
Updated types and interfaces for better clarity

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

changeset-bot · 2024-04-02T15:25:46Z

🦋 Changeset detected

Latest commit: fb852c6

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

codspeed-hq · 2024-04-02T15:29:35Z

CodSpeed Performance Report

Merging #2632 will not alter performance

_{Comparing mnn/connection-improvements (fb852c6) with main (1c29556)}

Summary

✅ 9 untouched benchmarks

joaquim-verges

looks fine, few questions.

also is this a feat: or a refactor: ?

joaquim-verges · 2024-04-02T15:36:50Z

packages/thirdweb/src/wallets/wallet-connect/index.ts

+  }
+
+  // limit optional chains to 10
+  const optionalChains = (options?.optionalChains || []).slice(0, 10);


was this a limitation from WC? or our choice?

Wallet connection does not work if send too many optional chains ( this happens in our dashboard - we also limit the optional chains in v4 for this reason )

joaquim-verges · 2024-04-02T15:39:15Z

packages/thirdweb/src/wallets/injected/index.ts

+    if (accounts[0]) {
+      const newAccount = {
+        ...account,
+        address: getAddress(accounts[0]),


is there an assumption that the 'new' account will always be accounts[0] ?

also kind of weird to spread the old account functions and just override the address, but i guess in this case it'll work

Yes - the first one is the "active" account if the wallet has multiple accounts (ex: metamask )

Re: the spread, We can also simply mutate the address property - but if we do that - the account object will not trigger a re-render in react - so we clone the object and change the address since that's the only change we need to make

Bumps [knip](https://github.com/webpro-nl/knip/tree/HEAD/packages/knip) from 5.17.4 to 5.18.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/webpro-nl/knip/releases">knip's releases</a>.</em></p> <blockquote> <h2>Release 5.18.2</h2> <ul> <li>Improve re-export handling (9ccefb34)</li> <li>Add identifier to trace for re-export from entry file (b0b8b3d3)</li> <li>Move tagged export logic into reusable handler (1dff2db4)</li> <li>Update docs (2f91c8d0)</li> <li>Update <code>@ericcornelissen/bash-parser</code> to 0.5.3 (<a href="https://github.com/webpro-nl/knip/tree/HEAD/packages/knip/issues/674">#674</a>) (9a7ffac2)</li> </ul> <h2>Release 5.18.1</h2> <ul> <li>Refactor and improve dep graph naming/typing (28f05f0a)</li> <li>Speed up (de)serialized and add test (deb3b9c5)</li> <li>Move cli tests involving stdout to separate folder (978674fc)</li> <li>Update and caretify dependencies (e14f6d24)</li> <li>Fix link to screenshot (c2f9507e)</li> </ul> <h2>Release 5.18.0</h2> <ul> <li>Don't report issues when using --trace (d8e9719b)</li> <li>Emphasize --debug and --trace on troubleshooting page (bcb5e93f)</li> <li>Add --performance screenshot (d0d01082)</li> <li>Integration test knip --cache for eslint as well (443b8be0)</li> <li>Dogfoodin' is underrated (7115889b)</li> <li>Update docs (0651144c)</li> <li>Test against latest TS 5.5 rc (f7aba23c)</li> <li>Add test for imports-self (closes <a href="https://github.com/webpro-nl/knip/tree/HEAD/packages/knip/issues/663">#663</a>) (736b0a25)</li> <li>Major refactor of dep graph for trace feature (ad16689b)</li> <li>Use cwd as default base in <code>toAbsolute</code> (678f47ab)</li> <li>Fix tsup entries are production entry files (4d839d8f)</li> <li>Don't need to cache package.json (da33b9c4)</li> <li>Better explain <code>ignoreBinaries</code> configuration option (<a href="https://github.com/webpro-nl/knip/tree/HEAD/packages/knip/issues/670">#670</a>) (8470505f)</li> <li>Update funding options (81cf806d)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/webpro-nl/knip/commit/021e9bbd8eff978b474292cef0c8645cc7e851af"><code>021e9bb</code></a> Release 5.18.2</li> <li><a href="https://github.com/webpro-nl/knip/commit/9ccefb34693911d284904fecea66a6ff70f283fa"><code>9ccefb3</code></a> Improve re-export handling</li> <li><a href="https://github.com/webpro-nl/knip/commit/b0b8b3d3457cf0a022f4bd4bdead997fb2e6d905"><code>b0b8b3d</code></a> Add identifier to trace for re-export from entry file</li> <li><a href="https://github.com/webpro-nl/knip/commit/1dff2db443484be7daadf15b638be9556ca26ef5"><code>1dff2db</code></a> Move tagged export logic into reusable handler</li> <li><a href="https://github.com/webpro-nl/knip/commit/9a7ffac20e82e035b5bb5fc0587e7d4bf0ba1513"><code>9a7ffac</code></a> Update <code>@ericcornelissen/bash-parser</code> to 0.5.3 (<a href="https://github.com/webpro-nl/knip/tree/HEAD/packages/knip/issues/674">#674</a>)</li> <li><a href="https://github.com/webpro-nl/knip/commit/d9f8fd9c903d8ad189d0bd4d87af474ac66ca8fc"><code>d9f8fd9</code></a> Release 5.18.1</li> <li><a href="https://github.com/webpro-nl/knip/commit/28f05f0ad314918d9f6ea0e5c1719d50ebffd9ee"><code>28f05f0</code></a> Refactor and improve dep graph naming/typing</li> <li><a href="https://github.com/webpro-nl/knip/commit/deb3b9c56f1d0711ea75af9694e2161d0e69b2a7"><code>deb3b9c</code></a> Speed up (de)serialized and add test</li> <li><a href="https://github.com/webpro-nl/knip/commit/978674fc5f492bebeb77a2c3a023ea16dd326f06"><code>978674f</code></a> Move cli tests involving stdout to separate folder</li> <li><a href="https://github.com/webpro-nl/knip/commit/e14f6d24caf8fff9cd3e825cde65bb7a842d0a6a"><code>e14f6d2</code></a> Update and caretify dependencies</li> <li>Additional commits viewable in <a href="https://github.com/webpro-nl/knip/commits/5.18.2/packages/knip">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=knip&package-manager=npm_and_yarn&previous-version=5.17.4&new-version=5.18.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>  --- ## PR-Codex overview This PR updates dependencies in `package.json` and `pnpm-lock.yaml`, focusing on the `knip` package version. ### Detailed summary - Updated `knip` package version from `5.17.4` to `5.18.2` - Updated `@ericcornelissen/bash-parser` version - Updated `picocolors` version > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

MananTank added 2 commits April 2, 2024 20:53

connection improvements

e0b0b5e

add changeset

bc2ca74

MananTank requested a review from a team April 2, 2024 15:25

joaquim-verges approved these changes Apr 2, 2024

View reviewed changes

update account in wallet

fb852c6

MananTank added this pull request to the merge queue Apr 2, 2024

Merged via the queue into main with commit d89f009 Apr 2, 2024

MananTank deleted the mnn/connection-improvements branch April 2, 2024 16:08

jnsdls mentioned this pull request Apr 2, 2024

Version Packages #2600

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Wallet connection improvements #2632

feat: Wallet connection improvements #2632

Uh oh!

MananTank commented Apr 2, 2024 •

edited by pr-codex bot

Loading

Uh oh!

changeset-bot bot commented Apr 2, 2024 •

edited

Loading

Uh oh!

codspeed-hq bot commented Apr 2, 2024 •

edited

Loading

Uh oh!

joaquim-verges left a comment

Uh oh!

joaquim-verges Apr 2, 2024

Uh oh!

MananTank Apr 2, 2024 •

edited

Loading

Uh oh!

joaquim-verges Apr 2, 2024

Uh oh!

MananTank Apr 2, 2024 •

edited

Loading

Uh oh!

Uh oh!

feat: Wallet connection improvements #2632

feat: Wallet connection improvements #2632

Uh oh!

Conversation

MananTank commented Apr 2, 2024 • edited by pr-codex bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR-Codex overview

Detailed summary

Uh oh!

changeset-bot bot commented Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

codspeed-hq bot commented Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #2632 will not alter performance

Summary

Uh oh!

joaquim-verges left a comment

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

MananTank Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

MananTank Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MananTank commented Apr 2, 2024 •

edited by pr-codex bot

Loading

changeset-bot bot commented Apr 2, 2024 •

edited

Loading

codspeed-hq bot commented Apr 2, 2024 •

edited

Loading

MananTank Apr 2, 2024 •

edited

Loading

MananTank Apr 2, 2024 •

edited

Loading