chore: bump react-hook-form, @hookform/resolvers, yup#18704
chore: bump react-hook-form, @hookform/resolvers, yup#18704
Conversation
Lemonexe
added
mobile
| isElectrum?: boolean; | ||
| }; | ||
|
|
||
| const signVerifySchema: yup.ObjectSchema<SignVerifyFields> = yup.object({ |
There was a problem hiding this comment.
Solve TS error at L76 (string | undefined not assignable to string for path, signature)
The problem arose when bumping @hookform/resolvers + react-hook-form, and then bumping yup did not help.
It took me a while to realize that the former two are not at fault, it's yup that was insufficiently typed (in both versions I tried!), and react-hook-form merely tightened its types to catch that.
| dispatch( | ||
| setBuySelectedReceiveAccount({ selectedReceiveAccount: undefined }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Only clear the cryptoValue | fiatValue field when you really selected a different asset, or different fiatCurrency.
This fixes an infinite render loop, that only manifested in @testing-library/react-native unit tests
(the app worked fine)
Note: I spend considerably time debugging this, trying to find the real root cause why this started happening after bumping react-hook-form to 7.55. I did not succeed 😞
This whole form is very complex, with a lot of watch→setValue interactions that are rather difficult to trace. I tried disabling many such interactions, and validations, to at least see if I can break the cycle, but to no avail. I also tried:
- wrapping the hook in
renderHookWithStoreProviderAsyncin aFormProvider - mocking
useDebounce - any of the three
setValueshere in cases'fiatCurrency'and'asset'are enough to crash - in the useQuotes.test, this line is enough to crash (without the other
setValues)
This issue sounds relevant, but I reproduced the same crash even in 7.55, not just 7.56
| const useAmountAndCurrencyFieldsChangeEffect = ({ setValue, getValues, watch }: TradingBuyForm) => { | ||
| const dispatch = useDispatch(); | ||
| const prevNetworkId = useRef<string | undefined>(undefined); | ||
| const prevCryptoId = useRef<CryptoId | undefined>(undefined); |
There was a problem hiding this comment.
using cryptoId I can distinguish tokens of the same network and solve #18677 (review)
jbazant
left a comment
jbazant
left a comment
There was a problem hiding this comment.
Mobile Trade part looks great. Thank you for the solution.
4 participants
Description
Replacement for closed #18677, because I needed to rename branch (if you're curious, see #18700)
react-hook-formand related packages (because they're QA'd together).Related Issue
Resolve #18090
Screenshots
Mobile trading form: don't reset amounts when asset or fiat currency is changed to the same one:
mobile trading form reset.webm
🔍🖥️ Suite web test results: View in Currents
🔍🖥️ Suite desktop test results: View in Currents
🔍🖥️ Suite native android test results: View in Currents