Numeric string fields #2643
Numeric string fields #2643
Conversation
… state 1. Update the form state comparison by converting numeric string fields to numbers, enabling correct dirty state evaluation 2. Add isNumericString utility function to identify numeric strings and exclude empty strings Fixes: Shopify#2629
1. Add tests to ensure isNumericString() returns true for valid numeric strings 2. Add tests to ensure isNumericString() returns false for non-numeric strings 3. Add tests to ensure isNumericString() returns false for non-string values Fixes: Shopify#2629
| @@ -138,7 +138,9 @@ export function useField<Value = string>( | |||
| return undefined; | |||
| }, | |||
| // eslint-disable-next-line react-hooks/exhaustive-deps | |||
| [state.value, ...dependencies], | |||
| [state.value, ...dependencies].map(value => | |||
There was a problem hiding this comment.
Can you please add some unit tests that showcase the behaviour that would have broken without this change?
There was a problem hiding this comment.
Yes @BPScott , I have added tests in utilities.test.ts. I'll add the changeset entry now, thanks!
There was a problem hiding this comment.
The tests in utilities.test.ts test the new helper that you've added. They don't test this line we're commenting on. It would be totally possible to undo the change on this line and all the tests would continue to pass. Yet it's this line that you're fixing a bug on, the addition of utility functions is kinda incidental.
I would like to see an test of useField that uses a numeric string, that fails if this line is as it is on main, and passes when this line is changed, so that I can understand the expected behaviour that you want to see.
Description
Fixes (issue #)
#2629