[RFC] onChange -> onInput, and don't polyfill onInput for uncontrolled components

[sebmarkbage]

onChange is a nicer name for what onInput does and the fact that it has propagated up to other high-level components as the default name is much nicer than onInput as a high level event name.

Generally it has been helpful for the many new-comers to React that don't know the DOM well (which is a lot more than the inverse). However, that doesn't change the fact that it can be confusing for people that are familiar.

Unfortunately, changing it now would cause confusion for everyone that already knows React.

The reason I'd like to change it now is because I'd like to get away from polyfilling it for uncontrolled components. This use case is filled with all kinds of imperative code which leads to edge cases. E.g. reading/setting e.target.value or reading/setting ref.value.

When you use controlled components you shouldn't need to touch them imperatively and therefore won't hit the edge cases. Ideally we should get away from reading from e.target.value and instead just pass the value directly to the event handler.

Proposal:

Controlled Components

• onInput: Polyfilled and works like onChange does today. It is allowed to over-fire many events even if nothing changed. May have special Fiber rules regarding synchronous flushing. Optional: Pass value as second arg.
• onChange: Works like onInput for one version but warns about being deprecated and suggests switching to onInput. In next version it works like the browser but still warns and tells you to use onInput forever.

Optional: Add a getter/setter on DOM .value in development mode and warn if this is used directly.

Uncontrolled Components

• onInput: Not polyfilled. Works however the browser works. Warns about browser differences if you don't also specify onClick, onKeyDown and/or onKeyUp. The warnings suggests implementing those listeners to cover more edge cases, or switch to a controlled component.
• onChange: Not polyfilled. Works however the browser works.

[NE-SmallTown]

> and the fact that it has propagated up to other high-level components

But onInput bubbles too.

I'd like to get away from polyfilling it for uncontrolled components

Could you link the src code of the polyfilling?

and instead just pass the value directly to the event handler

I think this is great,we can just change this,why to change the api?

Controlled Components onChange: Works like onInput for one version

What does the "one version" mean?maybe a typo?

In next version it works like the browser but still warns and tells you to use onInput forever

If have to do this,please just in DEV mode.

And about the Uncontrolled Components,if you add the onInput for it,maybe we should add something in the doc if ref prop be set too,because if we have onInput,there is no necessity to use ref in most cases.

[deecewan]

But onInput bubbles too.

I believe he means propagated as in 'a lot of people use it in higher level components'

one version

For the next version of react, it'll work as it currently does, but with a deprecation warning.
I would imagine that all warnings will be dev-only.

[NE-SmallTown]

@deecewan Thanks for your share.

I believe he means propagated as in 'a lot of people use it in higher level components'

Thanks,I misunderstand it.

For the next version of react, it'll work as it currently does, but with a deprecation warning.
I would imagine that all warnings will be dev-only.

The next sentence he uses the word "next version" but this sentence he uses "one version",that's why I confuse.

[syranide]

@sebmarkbage I've previously brought up separating uncontrolled and controlled inputs entirely and you guys seemed to generally agree, might this be a good time for that too or is that no longer relevant? I.e. <input> is uncontrolled and <FooBarInput> is a React-enhanced implementation with controlled behavior. Could possibly even be its own package? IMHO if we're going with removing attribute whitelists then not special-casing native inputs seems like a step in the same direction.

Regarding onChange vs onInput. Unless there are IME-issues with hiding onInput then that seems fine. But onChange seems like a generally good React naming convention for components that change, also, checkboxes/radios etc would still report their value change via onChange and not onInput right?

[sebmarkbage]

@syranide I think <input> vs <FooBarInput> is also an interesting avenue to explore but it is overloaded what that means. E.g. we could actually have the name <input> lazily load impl details for example or make that configurable in scope or a bunch of other things that user space could do. Therefore I kind of wanted to talk about the events in isolation from that so we can decouple those concerns.

[syranide]

@sebmarkbage I'm not entirely sure what you mean but it sounds like you've thought about it 👍

On topic, perhaps it would be an idea to settle for a unique event name, say onEveryChange, onImmediateChange, onValueChange, etc. That way there's no weird if-this-then-that behavior where the events behave differently depending on which mode the input is in and we're free to implement the behavior exactly as it best suits us, not having to worry about quirks in onInput (if that may be a problem). The original events are there as-is untouched, we just add another one. Or are we sure that our needs and onInput align 100%?

Perhaps it's not a big deal, but having access to the polyfilled event for uncontrolled inputs can probably come in handy too sometimes.

[sebmarkbage]

@trueadm Proposed the same thing (a new name).

onValueChange is a bit confusing with .checked changing. onEveryChange is nice but it would be nice to be able to fire the event even if nothing changed but something might have changed. That leaves the deduping to user code that already has the state available to do that rather than forcing the implementation to keep that state accessible and dedupe every time. It also simplifies the implementation burden.

I've been thinking about it for a bit. Right now we have very complex polyfills for all our events but really there's only a few things that we polyfill and override.

E.g. our onChange implementation just listens to more events than just onInput to fill the gap but browsers are fixing that and adding more support to onInput so presumably. It's not a huge implementation burden and it seems likely that we'll eventually just use onInput alone. In that world, it would seem like completely unnecessary overhead to have the custom event.

[trueadm]

@sebmarkbage if browser vendors are going to make onInput behave like our current onChange then let's change name like you said in your proposal. I had no idea that was going to be the case and it makes complete sense to align the awesome behaviour of onInput to what it will do in the future – when we'll have the day we can just remove our additional polyfills from onInput altogether.

There are a LOT of people using onInput in the React world (instead of onChange). I was actually one of those people too back when I wrote my first React app.

Can we warn in 15.x that we're going to change onChange to be onInput in 16? If we do, we'd also have to warn that onInput will change semantics too for 16.

[jquense]

I really the general concept of this change. It allows simplifying a really complicated polyfill while also actively encouraging folks to use the better controlled input pattern. I'd really encourage ya'll to please give a full major version for this to warn, and not add it in v15.6. The entire "form" library ecosystem in React is built on top of the behavior of onChange, and custom inputs most copy the onChange/value pattern.

[stryju]

@sebmarkbage

When you use controlled components you shouldn't need to touch them imperatively and therefore won't hit the edge cases. Ideally we should get away from reading from e.target.value and instead just pass the value directly to the event handler.

IMO losing access to e.target would be a regression - I'd like to keep having access to the element and it's props, which can be useful (access selection range, data-* props etc)

[mgol]

IMO losing access to e.target would be a regression - I'd like to keep having access to the element and it's props, which can be useful (access selection range, data-* props etc)

Another reason is access to input.validity which is needed for input validation.

[eps1lon]

• onChange: Works like onInput for one version but warns about being deprecated and suggests switching to onInput

The spec has a very clear example where both of these events make sense.

An example of a user interface involving both interactive manipulation and a commit action would be a Range controls that use a slider, when manipulated using a pointing device. While the user is dragging the control's knob, input events would fire whenever the position changed, whereas the change event would only fire when the user let go of the knob, committing to a specific value.
-- html.spec.whatwg.org/multipage/input.html#common-input-element-events

Maybe <input type="range" /> is a niche use case but it would be really nice if react-dom would implement onInput and onChange according to the spec.

https://codesandbox.io/s/j7074wxxvv illustrates the difference between current react-dom implementation and browser vendor implementation. Latest firefox and chrome are following the spec.

[trueadm]

I think in the long-term we probably want to move away from adding more event namespaces to ReactDOM, including the changing of onChange and onInput. A better approach would be to move this custom ReactDOM logic to a separate element/node. Doing so would allow us to move forward with higher-level abstractions without them conflicting with the existing and future DOM namespaces on the web platform.

[eps1lon]

@trueadm I don't think I understand what you mean by DOM namespace or event namespace. At least not in the context of onInput or onChange. I don't recognize a namespace here. They are part of the "standard spec" and not defined under some exotic namespace as far as I know.

I'm not even sure if namespaced events are part of the current spec.

[trueadm]

@eps1lon Sorry for the confusion. What I meant by namespace, was that they conflict with the already existing event names (a property on the Node object) in the spec. It's nothing to do with other specs or anything.

[sidati]

Wondering if you guys going to proceed in this onChange/onInput matter ?

[KasparEtter]

It seems to me that, instead of waiting for this issue to be resolved, you can simply write a custom input component, which exposes the DOM-based change event. This is how I implemented it (with TypeScript):

import { Component, createElement, InputHTMLAttributes } from 'react';

export interface CustomInputProps {
    onChange?: (event: Event) => void;
    onInput?: (event: Event) => void;
}

/**
 * This custom input component restores the 'onChange' and 'onInput' behavior of JavaScript.
 */
export class CustomInput extends Component<Omit<InputHTMLAttributes<HTMLInputElement>, 'onChange' | 'onInput' | 'ref'> & CustomInputProps> {
    private readonly registerCallbacks  = (element: HTMLInputElement | null) => {
        if (element) {
            element.onchange = this.props.onChange ? this.props.onChange : null;
            element.oninput = this.props.onInput ? this.props.onInput : null;
        }
    };

    public render() {
        return <input ref={this.registerCallbacks} {...this.props} onChange={undefined} onInput={undefined} />;
    }
}

Please let me know if you see ways to improve this approach or encounter problems with it. I'm still gaining experience with this CustomInput component. For example, checkboxes behave strangely. I either have to invert event.target.checked in the onChange handler while passing the value to the checkbox with checked or can get rid of this inversion when passing the value to the checkbox with defaultChecked but this then breaks that several checkboxes representing the same state in different places on the page keep in sync with the state. (In both cases, I didn't pass an onInput handler to the CustomInput for checkboxes.)

[ghost]

Any update on this issue?

[AgainPsychoX]

Bump