Non checkbox component can also have boolean values!

[pandaiolo]

Hi there

I updated to RHF v6, good job guys (although it was quite hard to work through all those breaking changes)

One of the issues that came up is when implementing the Controller component where we used rhf input before.

1. Issue with boolean values

An issue that took me some time to resolve, is with boolean-value component that are not checkboxes

Example:

<Controller
  control={control}
  defaultValue={false}
  name="Go to the beach or not?"
  as={
    <MuiSelect>
       <MuiMenuItem value={false} label="Stay home" />
       <MuiMenuItem value label="Go to the beach" />
    </MuiSelect>
   }/>

The above code does not work because of the following line in the Controller component:
https://github.com/react-hook-form/react-hook-form/blob/master/src/controller.tsx#L60

Because it is appending a checked prop in my Select component, instead of normal value:
https://github.com/react-hook-form/react-hook-form/blob/master/src/controller.tsx#L149

(it needs to be rewritten with manually passing onChange and value props)

Suggestion:

• Do not treat boolean value as exclusively restricted to checkbox components?
• Instead, maybe test the element to see if it correspond to a native checkbox element?
• Or, add both checked and value props (not sure if it wouldn't raise some warning though)?

2. Improvement suggestion

In our tests, we usually grab fields by their name prop, which is quite standard (for eg. in Capybara tests)./ The controller does not forward the name prop to the DOM element, which requires us to manually add it which is redundant (both on the Controller component, and the as or render content).

IS there a reason why you are not adding it? It would be a nice addition and would allow us to simplify our code that uses Controller component.

[bluebill1049]

Hey, @pandaiolo thanks for the detailed feedback.

1. please use render prop for checked value.
2. valid point please send an issue I will patch the name forwarding.

[pandaiolo]

Hey @bluebill1049 thanks for your quick answer!

I think point 1. is a real issue: a lot of people uses custom select, and a propotion of those, with boolean values. It will lead those people to use the Controller component like me, and fail miserably until they go inside the library code understand what is going on and why the value is undefined

(The onChange is alo behaving as non standard and trying to put a checked value:
https://github.com/react-hook-form/react-hook-form/blob/master/src/controller.tsx#L76 )

Please at the minimum add a clear note to the documentation that Controller will treat all boolean values like if it was addressed to a checkbox-like component, and that having a non-checkbox-like component you must 1. use the render prop and 2. manually implement the onChange event handler on the custom component.

That will save many developers troubles!

I am sticking to my opinion that this is quite debatable pattern :) The React spec allows non checkbox component to hold boolean values. It's a very valid use case and the normal use of Controller (with as prop) breaks those use cases without warning.

[bluebill1049]

Please at the minimum add a clear note to the documentation that Controller will treat all boolean values like if it was addressed to a checkbox-like component, and that having a non-checkbox-like component you must 1. use the render prop and 2. manually implement the onChange event handler on the custom component.

Can you please send us PR with above? I agree this will be useful and save many developers' trouble. In fact, let me add this, we determine whether its a checkbox via const isCheckboxInput = isBoolean(value); which is check against the default value.

[bluebill1049]

I am sticking to my opinion that this is quite debatable pattern :) The React spec allows non checkbox component to hold boolean values. It's a very valid use case and the normal use of Controller (with as prop) breaks those use cases without warning.

curious to hear what would you suggest, one of the main reasons we switch to adding support render was due to as's limitation.

[pandaiolo]

Sure, render is a fine workaround! (once we understand it's broken with as without explanation 💩 )

I dropped a few suggestions about point 1. in my original post, what do you think about them?

[bluebill1049]

one solution I can have in my head is to introduce defaultChecked

[bluebill1049]

defaultChecked will be set value to checked
defaultValue can stay as it is value boolean will still be boolean

i think this is better and more clear to the users.

[pandaiolo]

I am not sure I explained the issue correctly.

This issue not about checkbox, this is about all custom inputs that are not checkboxes (Like select in my example code). What do you think about those cases? Do you think it is normal that a custom select receives its value in a checked prop? (that is unsupported by Select component)

[bluebill1049]

I am not sure I explained the issue correctly.

This issue not about checkbox, this is about all custom inputs that are not checkboxes (Like select in my example code). What do you think about those cases? Do you think it is normal that a custom select receives its value in a checked prop? (that is unsupported by Select component)

Yes, i am aware of you concern and try to figure out a better solution here.

if your CustomSelect is return boolean, then you can set defautlValue to true/false and it will NOT set a checked prop.

if your are building a CheckedBox, then you can set defaultChcked then it will add checked prop.

[pandaiolo]

FWIW I made a sandbox to illustrate how following the docs lead to a weird situation that is difficult to understand (unless i'm doing something wrong!):

https://codesandbox.io/s/react-hook-form-with-ui-library-j41pg?file=/src/index.js

[bluebill1049]

I think everything has two sides, the obviously reason which I did that checked was to make developer's life easier, but the user can give me feedback a bit earlier (v5) since we have render now, it makes sense to remove that logic inside the code and info user to use render instead.

[pandaiolo]

What do you think about the following to make both checkbox and standard inputs work as expected with boolean values:

When the value is boolean:

• add both checked and value props to the as custom input?
• add both checked and value props to the event.target object of the onChange event?

[bluebill1049]

will document his down as well, so users aware this behavior.

[bluebill1049]

This will be the patched version: https://codesandbox.io/s/react-hook-form-with-ui-library-vzbfo?file=/src/index.js

[bluebill1049]

after few try and error above solution is not valid. we are better off remove checked with as

[pandaiolo]

Awesome @bluebill1049 thanks a lot! I think it also adds explicitness for the code that uses a checkbox, so it's also good there. No black magic ;)

[sgranadosa]

Have you an example a little bit more complete? I have issues on a Select with yes/no values, I can see the labels, but when i select 'no' is no triggered to validate but when I select 'yes' my form is validated.

I am trying this to make it work :
onChange={(event: any, selectedOptions: any) => {
if (selectedOptions.value === false) {
controllerField.onChange(selectedOptions.value as Boolean );
}