[ReactFormState] Numeric string fields with formatting are incorrectly interpreted as dirty #2629
Comments
|
Hi @LauraAubin A field with an initial value of "2.0" replaced by the user with "2" will be incorrectly considered dirty, as the strings are directly compared. We can instead of directly comparing the strings, we can cast numeric form fields to Numbers and then compare them. This way, the field would only be considered dirty if the underlying numeric values are different. When the field value is not a number, the comparison will result in NaN, which can be omitted as a valid dirty evaluation. We can Update the ReactFormState logic to cast numeric form fields to Numbers before comparing them, ensuring a more accurate dirty evaluation. Exclude cases where the field value is not a number, as NaN !== NaN if it is evaluated. I would love to take this one up. Can you assign me to this. |
|
@jayanth-kumar-morem go for it! Let me know if you need a hand or want to figure out a way to pair together. |
|
Sure!, Thanks @LauraAubin |
… 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
|
Hey @LauraAubin , I raised a PR for the issue. Kindly review it. |
Numeric fields which are of the type
stringare incorrectly comparing the initial and current value. For example, if you have a numeric field which passes an initial value of"2.0", and the user replaces the value in this field with"2", since FormState is directly comparing the strings, this field will be interpreted as dirty.One option would be to use numeric values for the form, but in some cases this is not ideal as the field has existing string formatting (such as a currency field replacing
2with2.00) and would cause us to have to reformat the field from when we pass in the initial value, and when we show the current value.In the Web codebase, we worked around this by converting our initial and current values and doing the dirty check ourselves:
Number("2.0") === Number("2"). I think we should be casting the numeric form fields to Numbers instead of comparing the exact strings. In cases where the field value is not a number,Number("A")will return NaN which can be omitted as a valid dirty evaluation (sinceNaN !== NaNif it is evaluated).The text was updated successfully, but these errors were encountered: