refactor(ConnectWalletForm): preserve key in errors #641
Conversation
github-actions
bot
added
the
area: popup
Extension builds preview
|
){kind=icon}
){kind=icon}
| const [autoKeyShareFailed, setAutoKeyShareFailed] = React.useState( | ||
| isAutoKeyAddFailed(state), | ||
| ); | ||
|
|
||
| const resetState = React.useCallback(async () => { | ||
| await clearConnectState(); | ||
| setErrors((_) => ({ ..._, keyPair: '', connect: '' })); | ||
| setErrors((_) => ({ ..._, keyPair: null, connect: null })); |
There was a problem hiding this comment.
I suggest renaming the parameter to prev or something else that suggests that this is the previous state. Clearer than _.
| setErrors((_) => ({ ..._, keyPair: null, connect: null })); | |
| setErrors((prev) => ({ ...prev, keyPair: null, connect: null })); |
| setAutoKeyShareFailed(false); | ||
| }, [clearConnectState]); | ||
|
|
||
| const toErrorInfo = React.useCallback( | ||
| (err?: string | ErrorWithKeyLike | null): ErrorInfo | null => { |
There was a problem hiding this comment.
Why do we have the error as string as well? Can't we pass the whole error directly? For example here:
web-monetization-extension/src/popup/components/ConnectWalletForm.tsx
Lines 114 to 124 in a012ce4
| keyPair: state?.status === 'error:key' ? toErrorInfo(state.error) : null, | ||
| connect: state?.status === 'error' ? toErrorInfo(state.error) : null, |
There was a problem hiding this comment.
Just for my information, state represents the transient state?
| const handleSubmit = async (ev?: React.FormEvent<HTMLFormElement>) => { | ||
| ev?.preventDefault(); |
There was a problem hiding this comment.
Why is the event optional now?
There was a problem hiding this comment.
Because now we manually trigger the submit (without the form also), like on "Accept consent" or "retry".
| walletAddressUrl: toErrorInfo(errWalletAddressUrl), | ||
| amount: toErrorInfo(errAmount), |
There was a problem hiding this comment.
We are mixing errors as exceptions and errors as value with the validateWalletAddressUrl and validateAmount. Not a big fan, but I understand why this approach.
It might be worth looking into neverthrow later on.
Earlier, we stored only localized message in
errors[foo]. Now we preserve thekeyand original error under{info}; and add the localized message under{message}.This will help with error handling based on key, and paves way to support retry add-key on retry-able errors (such as timed out waiting for login, wrong account was logged in, key-add consent accidentally declined etc.)
Part of #613