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

feat: 6 New Hooks, 7 New Connectors, Redux Integration, Upgrade Guide, Multi Account Support, Multi Blockchains, Zustand to 4.3.2, Updated/Styled Example Project, MM/CB Fixes #730

Closed
wants to merge 50 commits into from

Conversation

niZmosis
Copy link

@niZmosis niZmosis commented Dec 21, 2022

I have integrated Redux with the same format as zustand. Each connector has it's own store and associated hooks. There are two new packages, core-redux and store-redux.

Updated Zustand to 4.3.2
Added ability to add and switch chain when watching an asset.
Added multi account support, via accountIndex hook.
Added how to migrate from v6 to v8 in the Readme.
Added many chains to the configuration of the example-next package as well as removed deprecated chains.
Styled example project.

New Packages

  • core-redux
  • store-redux

New Connectors

  • BSC Wallet
  • Portis Wallet
  • TronLink Wallet (Tron)
  • Nami Wallet (Cardano)
  • Yoroi Wallet (Cardano)
  • Phantom Wallet (Solana)
  • Solflare Wallet (Solana)

New Hooks

  • useAddingChain()
  • useSwitchingChain()
  • useWatchingAsset()
  • useAccountIndex()
  • useENSAvatar()
  • useENSAvatars()

MetaMask Connector

  • Fix for MM disconnecting when switching chains on certain networks
  • Fix for MetaMask isActivating connector not setting on initial connection

Coinbase Wallet Connector

  • Fix for Coinbase isActivating connector not setting when user rejects request

Provider

  • Added hook for setting the selectedConnector. Passing no argument will reset the selectedConnector to either the defaultSelectedConnector if one was provided, or to the priorityConnector.

All features have been implemented in the example project.

Added hook for resetting selected connector to priority
Implemented hooks in example-next for selecting a connector
@vercel
Copy link

vercel bot commented Dec 21, 2022

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

Name Status Preview Comments Updated
web3-react ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 10, 2023 at 0:55AM (UTC)

@niZmosis niZmosis changed the title Implemented ability to select a connector in the Web3ReactProvider feat: Ability to select a connector in the Web3ReactProvider Dec 21, 2022
@niZmosis niZmosis changed the title feat: Ability to select a connector in the Web3ReactProvider feat: Ability to set the selectedConnector in the Web3ReactProvider Dec 21, 2022
setSelectedConnector will now reset when no params are passed
Cleaned up a little bit of code
Added watchAsset example to example-next
Added watchAsset to readme
@niZmosis
Copy link
Author

niZmosis commented Dec 22, 2022

There is an existing bug in the @web3-react/metamask package when changing chains where it may not activate after it switches chains. It is reproducible on the existing example, mostly when switching to the Optimism network. In the build from this PR, you will see Fuji does it every time. It seems to be for only certain chains. Coinbase Wallet is fine. Seems at one point when switching, its clearing the data like it should, but doesn't flag it as isActivating. All the changes in the PR as of now are good to go. If I find this existing MM bug, i'll add it to the PR.

Edit: There is an existing issue opened on this already

Edit: The issue is with the MM extension, and has been around for a while. MetaMask github

Edit: Added a temp fix for it in this PR untill MM actually fixes their extension.

@niZmosis
Copy link
Author

I put a lot of work into this, with no breaking changes. I have merged into the main quite a few times, and I feel like this PR is invisible. What's wrong with this build where it's not getting any attention from the maintainers? Where do you guys even see this project going? What are the future plans? All the things I have added are in the example, and you wouldn't know it but metamask and cb wallets are on redux while the others are on zustand to show the flexibility and if someone has redux in their project already they won't need to install zustand just to use this package. I've updated packages, made code more similar between connectors. Broke out code to make it cleaner. There is so much that can be done with this feature wise. The team should make a list of their roadmap for this package, it's been in beta for over a year. Why no detailed readme? Yes it's in beta and you'd probably have to go and redo the whole readme, but that's what needs to happen, it's part of progress. From here either my PR dies or if going to be accepted, I will add even more to the readme. Yes this is a big ass merge, but it can't be broken up, the are subtle changes that need to be in place for all this to work with the different connectors. I go through the issues and see what people need. For instance, some people when they activate a connector, they need the address or chain to do additional things in the same block of code and not wait for a rerender with a useeffect just to make an api call. That's why activate and connect eagerly return the stores state. Anyways enough with my rant, it's just that I put a lot of thought and effort into this. You guys gangbanged the shit out of the last PR so I know there are quite a few of you. Someone review this thing.

@grabbou
Copy link
Contributor

grabbou commented Apr 5, 2023

Hey!

Unfortunately, we discussed this PR and decided not to merge it going forward. I understand this is not the outcome that you expected, but we - as a team - had no visibility into your work and were not expecting such a PR to arrive. While I really appreciate your time spent on our project, we simply can't merge it for that reason alone.

To put more context into it, we don't have any items on the roadmap rather than maintaining the current state of web3-react by providing necessary fixes and small features that are primarily required by Uniswap. We are happy with the project's current state, and while I certainly see hooks such as useENSAvatar() a useful addition, they're out of scope for what we consider priorities right now.

Once again, I am really sorry to be closing such impressive PR. If you decide to contribute to our project again, you're more than welcome, and feel free to engage with me into discussions here on Github so that we can plan feature developments out and make sure that as soon as they're completed, they're merged right away.

@grabbou grabbou closed this Apr 5, 2023
@niZmosis niZmosis mentioned this pull request Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants