-
Notifications
You must be signed in to change notification settings - Fork 7
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
Enable ABI types and fix erros #104
Conversation
const address = getAddress(e.target.value); | ||
setTargetContract(address); | ||
} catch (e) { | ||
setTargetContract(undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work if you copy the address. But if for some reason you entered it manually, it would always reset the value to undefined.
We could show an error alert instead and simply assume that you will always copy addresses. But I would never change the user's value without proper feedback about what happened
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I rolled back the targetContract
type to string
and now I use as Address
where is needed.
Maybe for another ticket: I noticed that we are using our own isAddress
function instead of the provided by Viem. Is there any reason for this? I found out the Viem checker also checks for checksums.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware of viem's version, so it would be ideal if we switch into using it.
However, there's a great chance that people copy an address without the checksum, so I would be cautious with how strict we are with the checksum errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Np, I'll open a ticket for it 🎫
This PR tries to fix some Wagmi hooks that are not detecting the ABIs types.