react-hook-form / react-hook-form

📋 React Hooks for form state management and validation (Web + React Native)
https://react-hook-form.com
MIT License
41.57k stars 2.08k forks source link

Improve isEmpty check of validateField function #11180

Closed rvetere closed 11 months ago

rvetere commented 1 year ago

TLDR;

By increasing the priority of inputValue in the isEmpty check, we can have working validation with fields that only update the state (but not the DOM). PR

Background

First of all, this project is absolutely amazing! It's simply the best form library i ever worked with!

I've debugged the validateField method a lot now and I think the line "isEmpty" could be improved.

We are implementing a multi-select field with downshift.js and there we are creating custom onChange events that are correctly updating the internal form state of react-hook-form - everything great so far.

The problem is, we don't have a native select element in the DOM, representing a correct value in a classic way.

This makes react-hook-form think the field is empty.

Screenshot 2023-11-09 at 20 09 17

In that case, we have a value that is an array, and - as you can see, the field would have a valid value indeed - but still, react-hook-form thinks its empty.

Screenshot 2023-11-09 at 20 09 44

This is because the "ref" is not having a value in plain HTML:

Screenshot 2023-11-09 at 20 09 59

But, the last part where it checks that the value also could be an Array, is not failing:

Screenshot 2023-11-09 at 20 10 12

This forces us to do a very ugly workaround in our implementation:

Screenshot 2023-11-09 at 20 14 50

This "hack" is fixing the is-empty-check, but nobody in our company is going to understand why our MultiSelectDropdown must have this value on the "ToggleButton" ^^

It is just to suppress the ref-check.

I've created a PR that would (hopefully) improve this check: https://github.com/react-hook-form/react-hook-form/pull/11179

bluebill1049 commented 1 year ago

Thanks very much for the PR! what's your validateField function looks like? or are you refer to the required prop?

rvetere commented 1 year ago

Hi @bluebill1049 - i have created a codesandbox to show our case with downshift.js ;-) https://codesandbox.io/s/sharp-brahmagupta-4s8h6x?file=/src/index.js

In this sandbox, it creates the exact same state as we have it -> the form state is updated correctly, but the referenced DOM element has no value attribute:

Screenshot 2023-11-13 at 10 17 51
bluebill1049 commented 1 year ago

thanks @rvetere will check it later. I think for custom-required checks, validate function is ideal for such, but I will verify the use case.

bluebill1049 commented 11 months ago

parking in the todo column for now.

rvetere commented 11 months ago

Sorry i was in vacation the past 4 weeks - my colleagues have rewritten our "custom field implementation" in the meantime and we are not facing this issue anymore :)

bluebill1049 commented 11 months ago

kk good to hear ad thanks for let us know. 🙏