issue: onChange validation of arrays with a single value not rendering

[eg-bernardo]

Version Number

7.45.1

Codesandbox/Expo snack

https://codesandbox.io/s/array-validation-issue-repro-hqn27r

Steps to reproduce

1. Click "Submit"
2. Click on '[0]'
3. Click on '[1]'
4. Click "Submit"

Expected behaviour

1. Required error appears.
   ok
2. Required error goes away.
   error, required error stays
3. "At least one even number" error appears.
   error, required error stays even though validate get's called
4. Nothing changes.
   now form does update

What browsers are you seeing the problem on?

Firefox, Chrome

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[eg-bernardo]

It seems this issue was introduced on 7.45.1, 7.45.0 and before don't have it. It's still present in 7.45.2.

[leapful]

@eg-bernardo

The root cause is the checking field value with isNaN() at line 751.

In case field value is an array with only 1 item, for example, isNaN([1]) returns false because [1] will be transformed into 1 and it's a valid number. It still works normally on array with more than 1 item because isNaN([1, 2, 3]) returns true as it should be.

react-hook-form/src/logic/createFormControl.ts

Lines 749 to 751 in 109a12e

	isFieldValueUpdated =
	isNaN(fieldValue) ||
	fieldValue === get(_formValues, name, fieldValue);

@bluebill1049

May be a simple check Array.isArray() is enough to fix this issue. Do you see any problem with that fix ?

[bluebill1049]

thanks @leapful let's add the check 🙏 would you mind to send a PR on this?

[eg-bernardo]

If the isNaN is there to avoid issues with NaN === NaN not being true, wouldn't it be better to use (typeof fieldValue === "number" && isNaN(fieldValue)) instead? Or avoid it entirely by using Object.is(fieldValue, get(_formValues, name, fieldValue))?

[leapful]

@eg-bernardo

If the isNaN is there to avoid issues with NaN === NaN not being true, wouldn't it be better to use (typeof fieldValue === "number" && isNaN(fieldValue)) instead? Or avoid it entirely by using Object.is(fieldValue, get(_formValues, name, fieldValue))?

=> For checking updated value on array, it would be safer to use JSON.stringify() to compare string value.

@hichemfantar

Can confirm that 7.45.1 introduces some nasty bugs with array fields. One example is that a simple append doesn't work properly. It updates the array but then immediately resets the array to its default value.

=> Please open an new issue or discussion with a minimal re-producible codesandbox url if you find out something that could be a bug. It would help to avoid confusion on this current issue.

Edit: Perhaps RHF should have some automated tests that run before every release to catch these breaking bugs early. This is a pretty huge bug for a patch release and could be avoided with proper tests.

=> E2E tests are already implemented in here https://github.com/react-hook-form/react-hook-form/tree/master/cypress. Feel free to submit a PR with your test cases. Open source libraries always needs contribution from the community to make it better since no one can cover all test cases from real world.

[bluebill1049]

Edit: Perhaps RHF should have some automated tests that run before every release to catch these breaking bugs early.
This is a pretty huge bug for a patch release and could be avoided with proper tests.

This type of statement is completely effortless and blind at the same time.

[eg-bernardo]

@leapful
I don't think react-hook-form should have special handling for arrays unless useFieldArray is used. It should treat values as opaque. Also JSON.stringify could have issues with recursive objects and objects that have custom toJSON implementation.

I'm in the process of creating a PR, but I'm not being able to reproduce the issue in a test in src/__tests__/controller.test.tsx. Maybe I'll try with a cypress test.

[bluebill1049]

thanks for looking into this @eg-bernardo 🙏

[eg-bernardo]

I cannot reproduce it because this issue has been fixed by 3ca1701

I'll create a PR with the test to avoid future regressions.

[bluebill1049]

thanks @eg-bernardo i will try to release the patch this week.