feat: add alchemy accounts context #539
Conversation
moldy530
requested review from
rthomare,
dancoombs,
mokok123,
avasisht23 and
denniswon
as code owners
moldy530
force-pushed
the
moldy/prototype-context
branch
from
7b451ff to
dbca7a3
Compare
|
example usage can be found in alchemyplatform/embedded-accounts-demo#4 |
|
TODOS
|
moldy530
force-pushed
the
moldy/prototype-context
branch
from
dbca7a3 to
bed4a62
Compare
| return user; | ||
| }, | ||
| }, | ||
| queryClient |
There was a problem hiding this comment.
I had to do this because for w/e reason react-query exports hooks under modern or under legacy. aa-alchemy was using legacy and the demo app was using modern. doing it this way ensures a query client is always present
| ? Omit<CreateLightAccountParams, "signer" | "transport" | "chain"> | ||
| : Omit< | ||
| CreateMultiOwnerModularAccountParams, | ||
| "signer" | "transport" | "chain" |
There was a problem hiding this comment.
actually, we shouldn't omit the signer here and instead make it optional. that would allow someone to create a session key signer based account
There was a problem hiding this comment.
not gonna do this now actually as it requires customizing signer in a lot of other places. opting for simplicity over flexibility / power in this first approach
There was a problem hiding this comment.
if we want session keys, we can always add a hook for that later
There was a problem hiding this comment.
do we want this to be with the CreateModularAccountParams, since we made CreateMultiOwnerModularAccountParams different from the base modular account?
There was a problem hiding this comment.
ah for now, I think we just want MultiOwner and LA and not MultiSig or other MA types.
| export type CreateConfigProps = ConnectionConfig & { | ||
| chain: Chain; | ||
| sessionConfig?: AlchemySignerParams["sessionConfig"]; | ||
| signerConnection?: ConnectionConfig; |
There was a problem hiding this comment.
this can be used as an override if you have different rpc urls. This is most useful if devs are doing what they shouldn't be doing and putting their API keys in the front end
| children, | ||
| }: React.PropsWithChildren<AlchemyAccountsProviderProps>) => { | ||
| // Note: we don't use .tsx because we don't wanna use rollup or similar to bundle this package. | ||
| // This lets us continue to use TSC for building the packages which preserves the "use client" above |
There was a problem hiding this comment.
to expand a bit more, when I first prototyped this outside of this repo, I was initially using rollup to build. This was fine except that it wouldn't preserve the use client directive so some hooks weren't working well in nextjs.
We'll need to figure this out when we add UI components, but for hooks I think this is fine for now
There was a problem hiding this comment.
i assume this .ts vs. .tsx reason applies to the other client-side hooks? if so, we should leave comments there too?
There was a problem hiding this comment.
the hooks files don't actually have any jsx syntax in them so we don't have to do that there
moldy530
force-pushed
the
moldy/prototype-context
branch
5 times, most recently
from
597086e to
2891ade
Compare
| @@ -232,5 +232,14 @@ export class SessionManager { | |||
|
|
|||
| this.setSession({ type: "passkey", user }); | |||
| }); | |||
|
|
|||
| window.addEventListener("storage", (e: StorageEvent) => { | |||
There was a problem hiding this comment.
I didn't know this event existed. This will be very useful to me in Worth of Words, thanks for showing it!
There was a problem hiding this comment.
oh speaking of worth of words! would be good to understand from your lens as a potential consumer of this what we should focus on supporting next for your use case. can be a good way to dogfood this; ideally, we can replace most of your aa-sdk usage with hooks from this package
moldy530
force-pushed
the
moldy/prototype-context
branch
from
2891ade to
beb55c1
Compare
moldy530
force-pushed
the
moldy/prototype-context
branch
2 times, most recently
from
d84a1eb to
55f757f
Compare
moldy530
force-pushed
the
moldy/prototype-context
branch
from
55f757f to
5a88d2c
Compare
| ? Omit<CreateLightAccountParams, "signer" | "transport" | "chain"> | ||
| : Omit< | ||
| CreateMultiOwnerModularAccountParams, | ||
| "signer" | "transport" | "chain" |
There was a problem hiding this comment.
do we want this to be with the CreateModularAccountParams, since we made CreateMultiOwnerModularAccountParams different from the base modular account?
| // TODO: figure out how to handle this on the server | ||
| // I think we need a version of the signer that can be run on the server that essentially no-ops or errors | ||
| // for all calls | ||
| return useSyncExternalStore( | ||
| watchSigner(config), | ||
| () => config.signer, | ||
| // We don't want to return null here, should return something of type AlchemySigner | ||
| () => null |
There was a problem hiding this comment.
and the reason it can't run work on server is because the alchemy signer needs the turnkey iframe? have we chatted with them on server-side solutions?
There was a problem hiding this comment.
it's not just that, but server-side state becomes global to the application. that means you can run into weird issues where you accidentally log a user in server side and that has all your other requests from other users authenticated as that user
| children, | ||
| }: React.PropsWithChildren<AlchemyAccountsProviderProps>) => { | ||
| // Note: we don't use .tsx because we don't wanna use rollup or similar to bundle this package. | ||
| // This lets us continue to use TSC for building the packages which preserves the "use client" above |
There was a problem hiding this comment.
i assume this .ts vs. .tsx reason applies to the other client-side hooks? if so, we should leave comments there too?
Co-authored-by: Ajay Vasisht <43521356+avasisht23@users.noreply.github.com>
| const search = new URLSearchParams(window.location.search); | ||
| if (search.has("bundle")) { | ||
| signer.authenticate({ type: "email", bundle: search.get("bundle")! }); | ||
| } |
There was a problem hiding this comment.
ultra nit: but "bundle" is a keyword and might be worth turning into a const that we can update in case we want to modify it in the future more easily if our email redirect flow changes.
not necessary to do in this pr
There was a problem hiding this comment.
oh yea good shout
* docs(hooks): add initial react hooks docs * refactor: apply suggestions from code review Co-authored-by: Ajay Vasisht <43521356+avasisht23@users.noreply.github.com> --------- Co-authored-by: Ajay Vasisht <43521356+avasisht23@users.noreply.github.com>
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 introduces new React hooks and config options, enhances error handling, and adds client-side properties.
Detailed summary
AWAITING_EMAIL_AUTHstatus insigner/types.tsconfigwithrpcUrlandchaininsite/snippets/react/config.tsComponentWithSignerStatusandComponentWithUserinsite/snippets/react/useSignerStatus.tsxandsite/snippets/react/useUser.tsxNoAlchemyAccountContextErrorinpackages/alchemy/src/react/errors.tsgetBundlerClient,getUser,getSigner,getSignerStatusinpackages/alchemy/src/config/actions/tsconfig.jsonformoduleResolutionandmodulepackages/alchemy/src/signer/session/manager.tsClientOnlyPropertyErrorinpackages/alchemy/src/config/errors.tsComponentWithSignerandComponentWithBundlerClientinsite/snippets/react/useSigner.tsxandsite/snippets/react/useBundlerClient.tsxLogincomponent insite/snippets/react/login.tsxwatchUser,watchSigner, andwatchSignerStatusinpackages/alchemy/src/config/actions/useUseranduseBundlerClienthooks inpackages/alchemy/src/react/hooks/