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

drop Web3ReactProvider context provider in favor of zustand #621

Open
sakulstra opened this issue Jul 20, 2022 · 6 comments
Open

drop Web3ReactProvider context provider in favor of zustand #621

sakulstra opened this issue Jul 20, 2022 · 6 comments

Comments

@sakulstra
Copy link

sakulstra commented Jul 20, 2022

Hello, i'm currently looking into web3-react@v8 and wondering why /store uses zustand, but Web3ReactProvider uses context.
Wouldn't it make sense to make this a zustand store as well? Seems a bit weird to do zustand->hooks->context, when it could just be zustand i think.

We're already using zustand in the dapp and it'd be cool if web3-react would just be a slice we can import so it's together with other stores inside the redux dev tools.

@sakulstra
Copy link
Author

@NoahZinsmeister any thought about this?
Have been digging a bit through the code and i think it's non trivial as essentially the whole core package has to be rewritten.

The idea would essentially be to have a zustand slice which act very similar to the current context.

export const createWeb3ReactSlice = (connectors, set, get) => {
  // initialize listeners that alter store
  return {
    connectors, // caching connectors
    connector,
    chainId,
    accounts,
    isActivating,
    account,
    isActive,
    provider,
    ENSNames,
    ENSName
  }
}

On uniswap/interface as i see now a mixture of zustand/redux & context is used. Would probably be nice there if it was a single source of truth as well.
Also this would make the package usable outside of react I think 🤔


There also could still be a context component using zustand under the hood for ppl who want it to be in context.

@vm
Copy link
Contributor

vm commented Jul 25, 2022

We're trying to create a separation between the external API and the implementation details. Ideally, we should be able to switch the internal state management without any breaking changes. The only thing is right now we force passing actions through to each Connector manually, so it's definitely clear that we're using zustand.

@talentlessguy
Copy link

talentlessguy commented Aug 18, 2022

it has been discussed before here: #481

@sakulstra
Copy link
Author

sakulstra commented Aug 18, 2022

@talentlessguy didn't see, but also don't agree with the conclusion :) The doc sais that "this is the recommended way, if you want to use context", not "you should use context"

seems like the native react context is recommended

The store created with create doesn't require context providers.

My proposal was slightly different to your pr though, as in the end you're still using a context provider in <Web3Context.Provider. Ppl probably should still be able to use context by injecting the store into a context, just raising that they shouldn't be forced to.

The beauty of having only an optional context but a native zustand store would be that you can:

  • just use it as a slice in zustand and rely on it in another zustand slice (e.g. reactively changing the ens image when the address updates)
  • debug your whole application state in redux dev tools (which are compatible with zustand)
  • don't be coupled to react (as context is, but zustand is not)
  • have a slight performance boost (as react context is not really suitable for bigger dynamic objects due to missing context selectors which i guess is why redux removed context usage in v7)

@vm
Copy link
Contributor

vm commented Aug 18, 2022

We are just concerned with supporting these use cases if we ever decide to switch our internal data respresentations. If we decide to change the store in any way that would be a breaking change, which would be difficult for us to maintain.

@sakulstra
Copy link
Author

@vm understand your concern, but I think there are ways to not create a huge maintenance burden, while still allowing flexibility.

The Web3Context react context utilizing the store internally should still be the default go-to for most users where the unstable_slice or whatever could be the slice for experienced zustand users who are willing to handle a few breaking changes for the gain of more flexibility.

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

No branches or pull requests

3 participants