React: Show a warning when value is provided to a checkbox input

[srph]

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

A checkbox input can have a value prop which is most likely ignored (it should be checked).

What is the expected behavior?

I think we should throw a warning if a user provides a value instead of checked to a <input type="checkbox" />.

[aweary]

Thanks for the request @srph! I think this is a good idea. Currently, if you use value instead of checked with a controlled checkbox it _looks_ like its working but it's not. The input just remains uncontrolled and if its value is updated in state elsewhere it won't update in the DOM.

Would you like to submit a PR for this?

[Hypnosphi]

The value prop is helpful when you use a native form for sending data to server. It is responsible for the actual value that gets sent to the server when the checkbox is checked and the form is being submitted. So, for example, <input type="checkbox" name="foo" value="true"/> would get submitted as foo=true when checked, which could be exactly what your server expects. The default behavior is to submit foo=on which could possibly be not exactly what you want.

Of course, using native forms for submitting data isn't really common in single-page apps, but as far as I know React doesn't enforce the apps using it to be single-page.

I understand the issue with pseudo-controlled checkboxes though. Maybe the warning should arise when there is an onChange handler attached but no checked prop provided, regardless of the presence of value?

[aweary]

I understand the issue with pseudo-controlled checkboxes though. Maybe the warning should arise when there is an onChange handler attached but no checked prop provided, regardless of the presence of value?

I think this is exactly what the warning should do 👍

[Hypnosphi]

Ok, I think I'll make a PR then. Should the same kind of warning apply to other controllable elements (text inputs, selects, etc.)?

[Hypnosphi]

I'm not really sure that having a handler on an uncontrolled component is an anti-pattern. Why should it be one?

The docs say:

To write an uncontrolled component, instead of writing an event handler for every state update, you can use a ref to get form values from the DOM.

But is there any explanation?

[Hypnosphi]

Oh, I didn't notice there was already an open PR on that issue. The questions above are still to be discussed though

[athomann]

Okay, I'll adjust #9490 with a warning once I get clarification from @Hypnosphi's questions.

[belikemike21]

Hello,

I like to solve this issue and I'm beginner to React.
If there is any slack channel or IRC channel then I would be happy to join it.

[sahilsharma011]

@jackysatpal You can get in touch by going here

[morajabi]

If it's not done yet, I can take it.

[nhunzaker]

Circling back to this, we have two outstanding PRs to resolve this: https://github.com/facebook/react/pull/10666, https://github.com/facebook/react/pull/9490 however, I'm still not sure that this is viable. There are still false warning cases. Working from @aweary's direction:

I understand the issue with pseudo-controlled checkboxes though. Maybe the warning should arise when there is an onChange handler attached but no checked prop provided, regardless of the presence of value?

When I updated https://github.com/facebook/react/pull/9490 (locally, don't want to force-push @athomann's master), I noticed that we hit a failure here:

https://github.com/facebook/react/blob/9d483dcfd6aad53ce082d249845436a56eb39248/packages/react-dom/src/__tests__/ReactDOMInput-test.js#L331-L342

onChange actually isn't necessary for this test. I think this test is trying to circumvent an older set of warnings that were meant for text input types but incorrectly triggered for checkbox/radio inputs.

_But_, it is an existing use case, and there is probably code in the wild that listens to changes on checkbox/radios while relying on implicitly setting an uncontrolled checkbox to unchecked when defaultChecked or checked aren't specified.

Unless we are absolutely sure this is an anti-pattern, I don't think we should have a warning.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!