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

feat: add useLogout hook #566

Merged
merged 10 commits into from
Apr 18, 2024
Merged

feat: add useLogout hook #566

merged 10 commits into from
Apr 18, 2024

Conversation

avasisht23
Copy link
Contributor

@avasisht23 avasisht23 commented Apr 11, 2024

Pull Request Checklist


PR-Codex overview

This PR adds a new useLogout hook to facilitate user logout functionality in the Alchemy package.

Detailed summary

  • Added useLogout hook in alchemy package for user logout
  • Updated useAddPasskey to include mutation handling
  • Created ComponentWithLogout in site/snippets/react/useLogout.tsx
  • Updated metadata and content in site/react/useLogout.md

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@avasisht23 avasisht23 changed the base branch from main to ajv/add_passkey_hook April 11, 2024 06:45
Copy link
Collaborator

@moldy530 moldy530 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about adding a doc to this PR.

Let's get in the habit of adding docs with code changes

packages/alchemy/src/react/hooks/useDisconnectSigner.ts Outdated Show resolved Hide resolved
packages/alchemy/src/react/hooks/useDisconnectSigner.ts Outdated Show resolved Hide resolved
@avasisht23 avasisht23 force-pushed the ajv/add_disconnect_signer_hook branch from e5ad797 to 7144c68 Compare April 12, 2024 16:55
@avasisht23 avasisht23 changed the title feat: add useDisconnectSigner hook feat: add useLogout hook Apr 12, 2024
@avasisht23 avasisht23 force-pushed the ajv/add_disconnect_signer_hook branch from 7144c68 to 00c9eb0 Compare April 12, 2024 18:18
@avasisht23 avasisht23 force-pushed the ajv/add_passkey_hook branch from 5ca33c5 to 62dc715 Compare April 12, 2024 18:58
@avasisht23 avasisht23 force-pushed the ajv/add_disconnect_signer_hook branch from 00c9eb0 to 58597f8 Compare April 12, 2024 18:58
moldy530
moldy530 previously approved these changes Apr 12, 2024
@avasisht23 avasisht23 force-pushed the ajv/add_passkey_hook branch from 62dc715 to 3d33dbe Compare April 15, 2024 02:31
@avasisht23 avasisht23 force-pushed the ajv/add_disconnect_signer_hook branch 2 times, most recently from 4ab6722 to 7cb782b Compare April 15, 2024 16:08
@avasisht23 avasisht23 force-pushed the ajv/add_passkey_hook branch from 69b045b to b4d0280 Compare April 16, 2024 05:39
@avasisht23 avasisht23 force-pushed the ajv/add_disconnect_signer_hook branch from 33413f4 to 57dacd7 Compare April 16, 2024 05:39
rthomare
rthomare previously approved these changes Apr 16, 2024
packages/alchemy/src/react/hooks/useLogout.ts Outdated Show resolved Hide resolved
packages/alchemy/src/react/hooks/useLogout.ts Outdated Show resolved Hide resolved
site/snippets/react/useLogout.tsx Outdated Show resolved Hide resolved
packages/alchemy/src/react/hooks/useLogout.ts Show resolved Hide resolved
Base automatically changed from ajv/add_passkey_hook to main April 17, 2024 14:41
@avasisht23 avasisht23 dismissed stale reviews from rthomare and moldy530 April 17, 2024 14:41

The base branch was changed.

Copy link
Contributor

@denniswon denniswon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, all the hooks contain repetitive code, basically doing the same things. Any idea on better refactoring / reduced code repeats?

Also, per my inline comment, we should check from AlchemyAccountContext which state the user/account is in, and throw informative error messages instead of just throwing null exceptions or other generic error messages

} = useMutation(
{
mutationFn: async (params: UseAddPasskeyParams) => {
return signer!.addPasskey(params ?? undefined);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for all out hooks, we should check from AlchemyAccountContext which state the user/account is in, and throw informative error messages instead of just throwing null exceptions or other generic error messages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the useSigner hook will throw an error if there is no clientStore we can get the signer from (and thus the user, account, etc.).

let's chat more with Moldy when he's back. he actually advised here that i move this approach.

packages/alchemy/src/react/hooks/useLogout.ts Outdated Show resolved Hide resolved
@avasisht23
Copy link
Contributor Author

avasisht23 commented Apr 17, 2024

In general, all the hooks contain repetitive code, basically doing the same things. Any idea on better refactoring / reduced code repeats?

re: this, lemme refactor after getting this hooks stack. agree there's probably some factory method i can create and pass a mutation function unique to the hook for.

@avasisht23 avasisht23 force-pushed the ajv/add_disconnect_signer_hook branch from 57dacd7 to fb55de7 Compare April 17, 2024 15:05
@denniswon
Copy link
Contributor

@avasisht23 if we are fast-following reviewed comments/TODOs, would be great to attach asana links to the tasks for each items to be followed. Otherwise those items would be hard to track.

@avasisht23 avasisht23 merged commit a64cf7f into main Apr 18, 2024
@avasisht23 avasisht23 deleted the ajv/add_disconnect_signer_hook branch April 18, 2024 17:52
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

Successfully merging this pull request may close these issues.

4 participants