feat: refactor network provider#144
Conversation
✅ Deploy Preview for lifted-alberto ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| expireInSecs, | ||
| }: MultisigDefaults) => { | ||
| const res = await n?.account.multisigSetDefaults({ | ||
| return await n?.account.multisigSetDefaults({ |
There was a problem hiding this comment.
Simple code inlining changes.
| const legacy = legacy_urls?.map(url => { | ||
| const n = new Network(url, anonymous) | ||
| n.apply([Account, Events]) | ||
| return n | ||
| }) |
There was a problem hiding this comment.
END was dropped. Only care about Manifest.
| const initialNetworks: NetworkInfo[] = [ | ||
| { | ||
| name: "Manifest Ledger", | ||
| url: "/api", | ||
| filter: "alberto", | ||
| }, | ||
| { | ||
| name: "Manifest Ledger (Alpha 1)", | ||
| url: "/api/legacy", | ||
| filter: "legacy", | ||
| }, | ||
| ] | ||
|
|
||
| const networksMap: Map<NetworkId, NetworkInfo> = new Map( | ||
| initialNetworks.map((network, index) => [index, network]), | ||
| ) |
There was a problem hiding this comment.
Simplified the network creation boilerplate. The network Map is created automatically from the initial network list.
|
|
||
| const devDomains = ["localhost", "liftedinit.tech"] | ||
|
|
||
| export const useNetworkStore = create<NetworksState & NetworksActions>( |
There was a problem hiding this comment.
Simplified some code here as well.
| const activeNetwork = get().getActiveNetwork() | ||
| return Array.from(get().networks.values()).filter( | ||
| ({ filter, parent }) => | ||
| filter === "legacy" && parent?.includes(activeNetwork.name), | ||
| ) |
There was a problem hiding this comment.
Only return legacy networks matching the parent
|
I can't run the pre-commit tests because of a glibc compatibility issue. Can someone run those, please? |
|
Checking CI issue... |
Codecov Report
@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 34.08% 34.43% +0.35%
==========================================
Files 88 88
Lines 2265 2262 -3
Branches 659 654 -5
==========================================
+ Hits 772 779 +7
+ Misses 1482 1472 -10
Partials 11 11
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Okay, I fixed the tests. Alberto needs some love. |
|
|
||
| export function useSaveWebauthnCredential() { | ||
| const [network] = useNetworkContext() | ||
| const { command: n } = useNetworkContext() |
There was a problem hiding this comment.
Oh this was a little confusing... We were importing the query network just to use it's URL to create a command network? Why would we do that? Maybe we don't apply the IdStore module outside of this context?
There was a problem hiding this comment.
Yeah I'm not sure why it was made the way it was.
There was a problem hiding this comment.
In the end I had to revert this specific change because of #147
| const queryNetwork = new Network(url, anonIdentity) | ||
| queryNetwork.apply([Ledger, IdStore, Account, Events, Base]) | ||
| const cmdNetwork = new Network(url, identity) | ||
| cmdNetwork.apply([Ledger, IdStore, Account]) |
There was a problem hiding this comment.
Nope, there it is. Weird.
| return [queryNetwork, cmdNetwork, eventNetworks] as [ | ||
| Network, | ||
| Network, | ||
| Network[], |
There was a problem hiding this comment.
Part of me misses the chance to extend this to:
[
Network,
Network,
Network[],
Network[][],
]
Oh well. You win some, you lose some.
There was a problem hiding this comment.
New design is still an improvement imo.
| setActiveId: (id: NetworkId) => void | ||
| updateNetwork: (id: NetworkId, n: NetworkParams) => void | ||
| getActiveNetwork: () => NetworkInfo | ||
| getLegacyNetworks: () => NetworkInfo[] | ||
| getNetworks: () => Map<NetworkId, NetworkInfo> | ||
| createNetwork: (n: NetworkInfo) => void | ||
| updateNetwork: (id: NetworkId, n: NetworkInfo) => void | ||
| deleteNetwork: (id: NetworkId) => void |
There was a problem hiding this comment.
Thanks for cleaning up NetworkStore. Zustand pretends to be all "light weight" but you can actually get pretty gnarly pretty fast. I think we should've treated it more like a closed module and been more careful with the interface or even used something like useReducer so an action in one part of the app (changing a Network) could impact the state of a different part (the current identity). It's probably not too late, but it would be a substantial refactor.
There was a problem hiding this comment.
The separation between Context and Store is a little weird at the moment.
2 participants
Reconcile code from liftedinit/gwen#75 and automate some boilerplate.