search

masa-finance / masa-react

Masa React Framework
https://masa.finance
MIT License
10 stars 5 forks source link

Update WAGMI, viem and RainbowKit #590

Open H34D opened 1 month ago

H34D commented 1 month ago

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

juanmanso commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

juanmanso commented 1 month ago

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

juanmanso commented 1 month ago

🟢 [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 commented 1 month ago

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 commented 1 month ago

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

juanmanso commented 1 month ago

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

Image

I'm gonna rely on Rainbowkit's migration guide

juanmanso commented 1 month ago

After running

yarn upgrade @rainbow-me/rainbowkit@2 wagmi@2 viem@2.x

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 commented 1 month ago

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 commented 1 month ago

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

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:

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 commented 1 month ago

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 commented 1 month ago

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

juanmanso commented 1 month ago

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:

juanmanso commented 1 month ago

🟢 [UPDATE] Got story to work with new provider

NEXT STEPS

Code is available here: e1a5f22

juanmanso commented 1 month ago

🟢 [UPDATE] Splitting work with @obasilakis

Gonna split the work as such:

juanmanso commented 1 month ago

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 commented 1 month ago

🟢 [EOD UPDATE]

Changes have been pushed to this branch:

Added Masa and MasaTestnet

Demo

Syntax errors

Image

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