Skip to content
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

[feature] config options for adding safe connector in defaultWagmiConfig #2483

Open
ChewySwap opened this issue Jun 30, 2024 · 5 comments · May be fixed by #2512
Open

[feature] config options for adding safe connector in defaultWagmiConfig #2483

ChewySwap opened this issue Jun 30, 2024 · 5 comments · May be fixed by #2512

Comments

@ChewySwap
Copy link

What problem does this new feature solve?

Currently as it stands we have to use wagmi's createConfig in order to use safe connector with web3modal. If a developer wants to use embedded wallets in addition to enabling safe connector so their app is Safe App enabled then it's not possible given the current config setup.

Describe the solution you'd like

I propose adding an optional field in defaultWagmiConfig to 'enableSafe' connector as well as another optional RegExp[] field for allowedDomains.

Copy link

linear bot commented Jun 30, 2024

@ChewySwap ChewySwap linked a pull request Jul 5, 2024 that will close this issue
8 tasks
@chris13524
Copy link
Member

chris13524 commented Jul 8, 2024

What do you think of the change in this PR, specifically this line? https://github.com/WalletConnect/web3modal/pull/2119/files#diff-34db059aa947ae724971eeeae0474109c902fd949914ed3b8761fbb2450684bbR44

Idea is to allow any custom connectors to be passed, vs a specific enableSafe method.

@ChewySwap
Copy link
Author

I like the idea of being able to pass custom connectors, the more extensibility the better of course. But I wouldn't really consider Safe connector a custom connector. At the same time sticking with what seems to be the logic of defaultWagmiConfig, to me it doesn't make sense that Safe connector is missing from there. Wagmi has 6 available connectors currently:

4 out of 6 are already supported by defaultWagmiConfig, just makes sense to at least have an option for Safe even if it's not enabled as default.

@chris13524
Copy link
Member

There are also use cases where developers want to pass custom/out-of-tree connectors, for example a Magic connector.

coinbaseWallet and the other connectors I would consider as special cases because they are enabled by default and there is no additional required config; we just handle it all, enabling support for as many wallets as possible out-of-the-box.

I suppose I would need to understand more what the Safe connector does, but right now I'd be in-favor of not adding an enableSafe flag in order to keep the API simple, and it would be easy to pass the connector yourself as a custom connector.

@ChewySwap
Copy link
Author

There are also use cases where developers want to pass custom/out-of-tree connectors, for example a Magic connector.

coinbaseWallet and the other connectors I would consider as special cases because they are enabled by default and there is no additional required config; we just handle it all, enabling support for as many wallets as possible out-of-the-box.

I suppose I would need to understand more what the Safe connector does, but right now I'd be in-favor of not adding an enableSafe flag in order to keep the API simple, and it would be easy to pass the connector yourself as a custom connector.

It's a connector for Safe Apps SDK for enabling your app to run inside of Gnosis Safe Multi-sig smart contract wallet (or forks of gnosis safe) as a "Safe App" the only other requirement is to edit your CORS headers for manifest.json in your dapp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants