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

Update WAGMI, viem and RainbowKit #590

Open
H34D opened this issue Jul 30, 2024 · 18 comments · May be fixed by #514
Open

Update WAGMI, viem and RainbowKit #590

H34D opened this issue Jul 30, 2024 · 18 comments · May be fixed by #514
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request task For general tasks tech debt Tech Debt Cleaning

Comments

@H34D
Copy link
Contributor

H34D commented Jul 30, 2024

The above libs are outdated and need to be updated to support newer wallets like phantom

@H34D H34D added task For general tasks tech debt Tech Debt Cleaning dependencies Pull requests that update a dependency file enhancement New feature or request labels Jul 30, 2024
@juanmanso juanmanso self-assigned this Jul 30, 2024
@juanmanso
Copy link
Contributor

Taking a look at the old PR to assert changes and see if they are salvageable or if I need to start from scratch

@juanmanso
Copy link
Contributor

Versioning breakdown

Current Latest
wagmi ^1.4.13 2.12.1
viem ^1.21.4 2.18.5
rainbowkit ^1.3.7 2.1.3

@juanmanso
Copy link
Contributor

Moved this back to Ready since I'm working on the higher priority bug

@juanmanso juanmanso assigned juanmanso and unassigned juanmanso Jul 31, 2024
@juanmanso
Copy link
Contributor

Resuming work. Gonna inspect #514 for ideas on the implementation of the update

@juanmanso
Copy link
Contributor

🟢 [UPDATE]

While reviewing the PR changes were way too many and hard to follow since there's just a single commit for the whole set of changes.

Instead what I ended up doing is a more hands-on approach where I start applying the changes myself to the latest stage of the repo. Still tinkering a bit but leaving an update of what I'm doing to keep communication fluid 💪

@juanmanso
Copy link
Contributor

Upgrading rainbowkit

Upgrading the @rainbow-me/rainbowkit dependency to its latest version throws the following warnings:

Image

This is coherent with their package.json's peerDependencies object, where viem needs to be 2.x and wagmi ^2.9.0.

An unexpected peerDependency appeared with @celo/rainbowkit-celo. However, they seem to not have updated their code as well since their rainbowkit dependency is still set to 1.x. We need to dig deeper into this but will be dismissed since it's not the focus of this ticket

@juanmanso juanmanso assigned juanmanso and unassigned juanmanso Aug 1, 2024
@juanmanso
Copy link
Contributor

Resuming work by doing the update by myself using #514 as reference

@juanmanso
Copy link
Contributor

Instead of troubleshooting in the dark fixing things like the one below

Image

I'm gonna rely on Rainbowkit's migration guide

@juanmanso
Copy link
Contributor

After running

yarn upgrade @rainbow-me/rainbowkit@2 wagmi@2 [email protected]

We got all the latest versions as shown on this message with a few minor version's offset:

Current Latest
wagmi ^1.4.13 2.12.2
viem ^1.21.4 2.18.7
rainbowkit ^1.3.7 2.1.3

@juanmanso
Copy link
Contributor

juanmanso commented Aug 1, 2024

By looking at viem's migration guide, it seems we have no breaking changes from its major version bump directly:

Note

This is mostly because we are not using viem directly but rather as a peer dependency for wagmi and rainbowkit

EDIT: However by looking at Simo's changes we might need to use some viem imports after all (link)

@juanmanso
Copy link
Contributor

juanmanso commented Aug 2, 2024

HOW TO DRILL DOWN THE CHANGES

The repo has the whole e2e flow by integrating stories that would act like the end user of the library (in our case, the SBT app).

So starting from the end user, how do changes affect the pipeline?

Stories

As on the SBT app, they import 2 key elements:

  • MasaProvider. It consists of 2 providers:
    • MasaWalletProvider
    • MasaStateProvider
  • Custom hooks that interact with the provider. What we care most in this scope are:
    • useWallet
    • useNetwork

MasaProvider

One important thing to mention is that MasaStateProvider makes use of TanStack/Query to manage the state and now it is a requirement for wagmi. Thus, since we cannot initiate 2 separated providers of TanStack/Query, we need to merge them.

The proposed solution on #514 was to just move it to:

  • Move QueryProvider to the scope of MasaWalletProvider
  • Create a wrapper provider for MasaWalletProvider that initiates:
    • WagmiProvider
    • QueryProvider
    • RainbowkitProvider
  • Leave only Client management logic to MasaStateProvider (via MasaClientProvider)

Further detail of the changes will be given later 💪

Custom hooks

From a high-level perspective, it seems that only syntax changes are required and minor tweaks such as how internal functions/hooks are used and called. Will dive into that later as well

@juanmanso
Copy link
Contributor

I'm a bit stuck with this task, gonna put it back to the ready column and take something else to get back to rhythm and come with a fresh mind to go at it again later 💪

@juanmanso juanmanso assigned juanmanso and unassigned juanmanso Aug 2, 2024
@juanmanso
Copy link
Contributor

Gonna do some pairing with @obasilakis here to move things forward 💪

@juanmanso
Copy link
Contributor

Pushed the changes I'm most certain to be good, have some others stashed locally but at least have them pushed so its visible for everyone at:

  • feat/MR-590/upgrate-rainbowkit-wagmi-viem

@juanmanso
Copy link
Contributor

🟢 [UPDATE] Got story to work with new provider

demo_wagmi_working.mov

NEXT STEPS

  • Fix storybook server errors (mainly due to some key files not being updated)
  • Filter networks through SupportedNetworks
  • Add Masa Network support
  • Test other functionalities

Code is available here: e1a5f22

@juanmanso
Copy link
Contributor

🟢 [UPDATE] Splitting work with @obasilakis

Gonna split the work as such:

  • @obasilakis is taking the syntax errors to be fixed
  • @juanmanso is taking the masa mainnet and testnet addition to the chains array

@juanmanso
Copy link
Contributor

Since this task no longer blocks masa-finance/masa-next-sbt#1444 due to the new findings and development cannot be finished in a couple of more hours, we dropped its priority and we moved it to the Ready column below the waterline

@juanmanso juanmanso self-assigned this Aug 14, 2024
@juanmanso
Copy link
Contributor

🟢 [EOD UPDATE]

Changes have been pushed to this branch:

  • feat/MR-590/update-rainbowkit-wagmi-viem

Added Masa and MasaTestnet

Demo
demo.mov

Syntax errors

Image

We are at 24 errors while running the storybook. Gonna continue fixing these next week 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request task For general tasks tech debt Tech Debt Cleaning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants