chore: store both MAv2 modes & use standard hook for minting 7702 in ui demo #1361
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
jakehobbs
force-pushed
the
jake/mav2-hooks-switch-mode-experiement
branch
2 times, most recently
from
5ffc7c0 to
9834b82
Compare
jakehobbs
force-pushed
the
jake/mav2-hooks-switch-mode-experiement
branch
from
48e1cbe to
0d5a958
Compare
jakehobbs
changed the title
jakehobbs
requested review from
moldy530,
rthomare,
dancoombs,
mokok123,
dphilipson,
linnall,
adamegyed,
howydev,
Zer0dot,
jaypaik,
Blu-J,
noam-alchemy and
archit2407
as code owners
account-kit/core/src/store/types.ts
Outdated
| @@ -81,15 +82,23 @@ export type StoreState = { | |||
| signer?: AlchemyWebSigner; | |||
| accounts?: { | |||
| [chain: number]: { | |||
| [key in SupportedAccountTypes]: AccountState<key>; | |||
| [T in SupportedAccountTypes]: T extends "ModularAccountV2" | |||
There was a problem hiding this comment.
i am slightly debating if it might be worth just nesting the accounts, configs, and clients for each type under a mode, even if they don't have different modes. it would just be default for all types other than MAv2. if people are okay w/ that, it would lead to less branching in a lot of places where we have to check the type to determine how we need to access the final objects.
There was a problem hiding this comment.
I think this is not only an easier (for future devs) but probably safer option.
| const cachedAccount = ( | ||
| type === "ModularAccountV2" | ||
| ? accounts[chain.id]?.["ModularAccountV2"]?.[mode] | ||
| : accounts[chain.id]?.[type] | ||
| ) as AccountState<TAccount>; |
There was a problem hiding this comment.
Is line 106 safe? We're casting it to a non-nullable type, but it seems like the actual thing can be undefined?
Lol maybe I've forgotten how js/ts works.
There was a problem hiding this comment.
i think it is always initialized with a defaultAccountState like this:
{
status: "DISCONNECTED",
account: undefined,
}
so shouldn't ever be undefined... i think the optional chaining here is redundant and the TS compiler knows it should always be defined.
|
|
||
| const accountPromise = (() => { | ||
| switch (type) { | ||
| case "LightAccount": | ||
| return createLightAccount({ | ||
| ...params, | ||
| ...cachedConfig, | ||
| ...(cachedConfig as OmitSignerTransportChain<CreateLightAccountParams>), |
There was a problem hiding this comment.
curious why now we have to do an as ... and didn't have to before?
There was a problem hiding this comment.
is there any way to do this without hardcoding? (i know that's what we did before so it might be OOS for this)
There was a problem hiding this comment.
let me work through that real quick
There was a problem hiding this comment.
i can simplify it if we're ok w/ a type assertion
Pull Request Checklist
yarn test)sitefolder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change)yarn lint:check) and fix any issues? (yarn lint:write)PR-Codex overview
This PR focuses on refactoring the account management system to support multiple account modes, specifically integrating a "default" and "7702" mode for various account types. It updates related components, hooks, and tests to accommodate this change.
Detailed summary
useModularAccountV2ClienttouseMAv2Client.walletTypewithaccountModein configuration and state management.modeparameter.accountMode.SupportedAccountModes.