(continued) Warning when input elements don't reference onChange - does React like event bubbling?

iansedano commented 2 years ago

Continuation of the following issue:

Inaccurate warning when value props is set without onChange.

Where a clear example in a new issue was requested.

Since it has gathered some popular comments since then, here is a new issue.

It is fully appreciated that this may be an annoying edge case that already has a workaround, but it still seems to crop up quite often in forums, Stackoverflow and while teaching React.

Issue

If someone wanted to structure their app making use of event bubbling, and as such, only requiring assigning one event handler to a parent element.

function App() {
  const [state, setState] = useState("test")
  return (
    <div onChange={(e) => setState(e.target.value)}>
      <input value={state} />
    </div>
  );
}

They will get a warning that the input element doesn't have an onChange handler.

https://codesandbox.io/s/onchange-warning-wzswb?file=/src/App.js

The warning can be suppressed by adding a blank function to the input element:

<input value={state} onChange={()=>{}}/>

Of course, in this example the same function used on the parent div can be applied to the input element, but if there were many input elements that could be handled by the same event handler, it means that you need to reference the event handler in all of the input elements.

<input onChange={changeHandler}/>
<input onChange={changeHandler}/>
<input onChange={changeHandler}/>

Or if you wanted to use event bubbling with only one reference to the handler, to suppress the warnings you can:

<input onChange={()=>{}}/>
<input onChange={()=>{}}/>
<input onChange={()=>{}}/>

TLDR of discussion in original issue

The main point of contention is the warning that pollutes the console. You can refactor to reference the onChange handler in all the input elements as is shown in the docs but that gets tedious and repetitive for large forms. Having to attach an empty function to suppress warnings is a workaround but is also tedious and repetitive.

The warning may be good for people learning how to think in React but the fact that it is sometimes inaccurate is problematic. That is, its not clear whether using event bubbling goes against best practice or not. The warning just says that there is no onChange handler which is not always true. Using event bubbling to send information back up the tree seems to go against the idea of using React state to do that job. In either case, both ways work and can be done within React, however, relying on event bubbling causes the warning on the console.

Is using event bubbling against the React way? Or is React just agnostic towards it?

Assuming its not against it, for complex forms that take advantage of event bubbling, the error pollutes the console and a way to silence this specific error (or perhaps specific errors in general) without having to flag every single input would be appreciated.

Potential solutions proposed

Disable this warning completely.
Implement a way to disable this particular warning.
Make a "beginner" category of warnings that can be disabled.
Make it so that event bubbling in this way is just not allowed (please no).
Document the event bubbling approach and make it clear that using this way will generate warnings because its against the React way.
Change the warning text to make clear that there may be an onChange handler further up the tree that it is not detecting, and/or that handlers should always be referenced by each input.

pangoleaf commented 2 years ago

+1 to this.

I'm learning React for the first time this week and this warning had me spend a while trying to work out if I was doing something wrong by using bubbling to handle changes to form fields, but from what I can tell it's a perfectly valid approach?

It doesn't feel like an edge case to me, it seems like using bubbling like this is a good way to manage forms with numerous fields.

As far as proposed solutions go, would it be possible to have the warning check if an input has an onChange handler anywhere further up the tree?

McGern commented 2 years ago

Another +1. I'm using onChange bubbling for lists of checkboxes and radio buttons (which surely must be a common practice).

In "old school" form setup (just html and javascript), you would use bubbling to reduce the number of event listeners added to the DOM, but knowing whether react (and/or modern browsers) take care of this regarding memory/performance/efficiency and best practice is to put it on each input would be nice.

facebook / react