chore: bump react-hook-form, @hookform/resolvers, yup#18677
chore: bump react-hook-form, @hookform/resolvers, yup#18677
Conversation
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.
Lemonexe
force-pushed
the
bump-hookform+resolvers+yup
branch
from
07e156d to
736ae5a
Compare
|
|
||
| if (asset?.networkId !== prevNetworkId.current) { | ||
| prevNetworkId.current = asset?.networkId; | ||
| setValue('cryptoValue', undefined, { shouldValidate: true }); |
There was a problem hiding this comment.
Fix an infinite render loop, that only manifested in tests (the app worked fine)
Lemonexe
force-pushed
the
bump-hookform+resolvers+yup
branch
from
736ae5a to
cfd9920
Compare
jbazant
left a comment
jbazant
left a comment
There was a problem hiding this comment.
The new behaviour is strange. I have no idea whats going on. The solution is probably to use same useRef with previous value of asset as you prosed with fiatCurrency.
| case 'asset': { | ||
| if (asset?.networkId !== prevNetworkId.current) { | ||
| prevNetworkId.current = asset?.networkId; | ||
| setValue('cryptoValue', undefined, { shouldValidate: true }); |
There was a problem hiding this comment.
issue: Moving this setValue inside the if is not correct. Now it does not clear value if you change asset e.g. from Ethereum to 1Inch (or anything on same network)
There was a problem hiding this comment.
Oh right! I hadn't thought about multiple assets on the same network, thanks.
In that case I shall find the real cause for the infinite render loop, and fix with keeping current behavior.
|
Closed because I needed to rename the branch. I renamed it via github UI, I hoped it would link this PR to new branch name, but I guess github does not support that 🤷 |
2 participants
Description
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