search

thirdweb-dev / js

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

fix(ts): import of coinbase sdk in vite #3562

Closed gregfromstl closed 3 months ago

gregfromstl commented 3 months ago

TL;DR

Corrected the import handling for CoinbaseWalletSDK in the coinbaseWebSDK.ts file.

What changed?

The import statement for CoinbaseWalletSDK was modified to handle cases where the import does not return a default function directly.

How to test?

Why make this change?

This change ensures that the CoinbaseWalletSDK import is handled correctly, thereby preventing potential import errors that could occur in various environments.

PR-Codex overview

This PR fixes coinbase wallet connections on Vite by addressing import errors with CoinbaseWalletSDK.

Detailed summary

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

vercel[bot] commented 3 months 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 2, 2024 1:09am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 2, 2024 1:09am
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 2, 2024 1:09am
changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 98a9e226fa4cecda00ddd4b7c675ec00d2139940

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 3 months 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 3 months 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 3 months ago

CodSpeed Performance Report

Merging #3562 will not alter performance

Comparing fix/vite-coinbase-import (98a9e22) with main (a1d6a3c)

Summary

✅ 9 untouched benchmarks

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 62.54%. Comparing base (a1d6a3c) to head (98a9e22).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3562 +/- ## ========================================== + Coverage 62.32% 62.54% +0.21% ========================================== Files 900 899 -1 Lines 69196 68950 -246 Branches 3671 3670 -1 ========================================== - Hits 43129 43123 -6 + Misses 25379 25139 -240 Partials 688 688 ``` | [Flag](https://app.codecov.io/gh/thirdweb-dev/js/pull/3562/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/3562/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 [a1d6a3c](https://app.codecov.io/gh/thirdweb-dev/js/commit/a1d6a3c93e9a7b0433076a3e321183b09cfc0054?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/3562/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | `61.96% <50.00%> (+0.25%)` | :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/3562?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev) | Coverage Δ | | |---|---|---| | [...es/thirdweb/src/wallets/coinbase/coinbaseWebSDK.ts](https://app.codecov.io/gh/thirdweb-dev/js/pull/3562?src=pr&el=tree&filepath=packages%2Fthirdweb%2Fsrc%2Fwallets%2Fcoinbase%2FcoinbaseWebSDK.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev#diff-cGFja2FnZXMvdGhpcmR3ZWIvc3JjL3dhbGxldHMvY29pbmJhc2UvY29pbmJhc2VXZWJTREsudHM=) | `36.71% <50.00%> (+0.24%)` | :arrow_up: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/thirdweb-dev/js/pull/3562/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thirdweb-dev)
github-actions[bot] commented 3 months ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 40.47 KB (0%) 810 ms (0%) 4.7 s (-2.43% 🔽) 5.5 s
thirdweb (cjs) 89.99 KB (0%) 1.8 s (0%) 9.1 s (+2.55% 🔺) 10.9 s
thirdweb (minimal + tree-shaking) 4.79 KB (0%) 96 ms (0%) 295 ms (-11.78% 🔽) 391 ms
thirdweb/chains (tree-shaking) 423 B (0%) 10 ms (0%) 139 ms (+138.58% 🔺) 149 ms
thirdweb/react (minimal + tree-shaking) 13.52 KB (0%) 271 ms (0%) 492 ms (-5.28% 🔽) 762 ms
graphite-app[bot] commented 3 months ago

Merge activity