Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use onInput instead of onChange #8550

Closed
wants to merge 1 commit into from
Closed

Use onInput instead of onChange #8550

wants to merge 1 commit into from

Conversation

mathiasbynens
Copy link

@mathiasbynens
Copy link
Author

I haven’t updated the CodePens that are linked, but would be happy to do so.

@vramana
Copy link

vramana commented Dec 10, 2016

@mathiasbynens Not related to the PR but why is onInput faster? Any resources?

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Dec 10, 2016

React's onChange doesn't work like the normal DOM onchange. It works more like oninput but also covers more cases.

Personally I think we should've gone with a different name to avoid confusion. The argument used in the early days of React went something like this:

  1. Tracking all possible changes is very important in React because of the controlled component model. If you don't track every change, then React resets the value and ignores that change.

  2. Newcomers to the platform intuitively think that onChange triggers for every change (for text fields, check boxes, select boxes etc.) and go for that first. We should optimize for newcomers rather than experts.

  3. Therefore, we should "fix" this event rather than force people to use another one.

In hindsight I probably would've added a new event name and then triggered a warning/error if someone tried to use the "broken" onChange event. Feel free to open up an RFC issue for further discussion if you think that is worth it.

@mathiasbynens
Copy link
Author

mathiasbynens commented Dec 11, 2016

@vramana See https://mathiasbynens.be/demo/input.

@sebmarkbage Thanks for clarifying! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants