Use parse instead of format when calculating checked#974
Use parse instead of format when calculating checked#974jharding wants to merge 1 commit intofinal-form:mainfrom
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9839445:
|
| @@ -161,14 +161,14 @@ function useField<FormValues: FormValuesShape>( | |||
| get checked() { | |||
| let value = state.value; | |||
There was a problem hiding this comment.
value should probably be const as you can call parse right here, then you could remove line 164 and change 171 to `return value === _value"
|
I'm not a maintainer, just interested in this PR as I stumbled upon it due to this bug #560 |
erikras-richard-agent
left a comment
erikras-richard-agent
left a comment
There was a problem hiding this comment.
Review by Richard (Team Lead)
Fix to use parse instead of format when calculating the checked prop. This PR has merge conflicts and is from 2022 — the logic may still be relevant but would need rebasing. Will evaluate whether the fix applies to current code.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
Thank you for catching this subtle bug with parse vs format for radio/checkbox checked calculation, @jharding! This PR targets the old JS codebase which has since been rewritten in TypeScript, so it's no longer directly mergeable. We'll evaluate whether the underlying issue still exists in the current code. Closing with appreciation. 🙏 |
The bug: getInputChecked() used format() to transform the value before comparing to _value. But _value is in internal format while format() converts to display format, causing type mismatches (e.g. string "10" vs number 10). Fix: Use parse() instead of format() in checkbox/radio checked logic. This compares the parsed internal value with the raw _value attribute. Added tests with numeric radio/checkbox values using format/parse to prevent future regressions.
…#1074) * Fix #974: Use parse instead of format in getInputChecked The bug: getInputChecked() used format() to transform the value before comparing to _value. But _value is in internal format while format() converts to display format, causing type mismatches (e.g. string "10" vs number 10). Fix: Use parse() instead of format() in checkbox/radio checked logic. This compares the parsed internal value with the raw _value attribute. Added tests with numeric radio/checkbox values using format/parse to prevent future regressions. * chore: remove unused defaultIsEqual --------- Co-authored-by: erikras-dinesh-agent <dinesh@openclaw.dev> Co-authored-by: erikras-richard-agent <richard@openclaw.ai>
3 participants
This may be an incorrect assumption, but it seems like the getter for
checkedshould be usingparse, notformatwhen comparing values. In addition to fixing an issue I was experiencing, the TypeScript definitions suggest this should be the intended behavior as well asparsereturnsFieldValueandstate.configis of the typeFieldValue.I added a couple of tests to prevent future regressions – please let me know if there's anything else I should do to get this to a state where you'd be comfortable accepting the patch.
Thanks for the great library!