-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: update the metamask sdk package to version "0.18.3" #3602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update the metamask sdk package to version "0.18.3" #3602
Conversation
🦋 Changeset detectedLatest commit: 4733017 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
…ask-sdk-package-to-version-0.15.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small change for dapp metadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
packages/connectors/src/metaMask.ts
Outdated
let sdk: MetaMaskSDK | ||
let sdkImport: Promise<typeof import('@metamask/sdk')> | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious why this was moved here? It should only be initialized once in the Connector instance (where it was before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes we can move it back, was initially testing different variation but eventually the scope shouldn't have impact.
dappMetadata: { | ||
name: 'Create Wagmi', | ||
}, | ||
infuraAPIKey: import.meta.env.VITE_INFURA_API_KEY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should expose this at all as not all consumers use Infura, and tend to bring their own RPC providers. Instead, can we have the ability to pass an arbitrary RPC URL to the MetaMask SDK? This is what we are doing for the other connectors, like WalletConnect: https://github.com/wevm/wagmi/blob/main/packages/connectors/src/walletConnect.ts#L212-L217
This means we can infer the RPC URL from the provided transports
from the Wagmi Config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, we have a similar way to configure rpcMap, will update accordingly.
Will review this in-depth later in the week and happy to merge after that. I was also wondering – has there been any efforts so far to mitigate bundle size? Saw that in the SDK changelog, but I am seeing no difference between 0.15.0
0.14.3
Reproduced with |
Not on the source itself, we are tackling that next. Currently we removed the sourcemaps that were creating high package size on npm and was potentially scaring people away (even though it is unrelated). We will have another update on the PR today to address your comments. |
Source maps don't necessarily affect the bundle size, devs can choose if they'd like to ship them or not. IMO the focus should be on the source code itself. For example I noticed the SDK installs react and react-native regardless of the environment used. Although this might be tree shaken I'm not sure why it would be needed. When installing Wagmi the console also shows some peer dependency issues that are from the SDK too. It would be nice to address that to have a clean install as well. |
…ask-sdk-package-to-version-0.15.0
…kage-to-version-0.15.0' into feat_update-the-metamask-sdk-package-to-version-0.15.0
…ask-sdk-package-to-version-0.15.0
Description
This PR updates the
MetaMask SDK
package to version0.18.3
, introducing significant improvements aimed at reducing the bundle size and enhancing the MetaMask connector's logic. This version marks a pivotal update where we've focused on optimizing the overall package size to ensure faster loading times and a more efficient integration process for developers. Alongside the SDK bundle size reduction, this PR addresses various bugs and enhances the SDK's functionality to operate seamlessly without the necessity for theextensionOnly=true
option.It is also crucial to highlight that
dappMetadata
has become mandatory for integration with this version. This change underscores our commitment to improving security and user experience by ensuring that dApps provide the necessary information for a safer and more transparent interaction.Furthermore, to expand the SDK's compatibility and usability on mobile devices, we have introduced support for custom RPC URLs. This enhancement allows for the direct incorporation of custom RPC URLs from the Wagmi Config, offering developers the flexibility to specify their network configurations, thereby facilitating a smoother integration with different blockchain networks.
These updates collectively contribute to a more robust, efficient, and versatile MetaMask SDK, setting a new standard for blockchain application development.
Additional Information
Before submitting this issue, please make sure you do the following.