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

Regression: onChange doesn't fire with defaultChecked and radio inputs #9988

Closed
ThisIsMissEm opened this issue Jun 16, 2017 · 12 comments · Fixed by #10156
Closed

Regression: onChange doesn't fire with defaultChecked and radio inputs #9988

ThisIsMissEm opened this issue Jun 16, 2017 · 12 comments · Fixed by #10156

Comments

@ThisIsMissEm
Copy link

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

What is the current behavior?
In React 15.6.1, this behaviour is changed; In 15.5.4, it fires the change event reliably.

15.6.1 - https://codesandbox.io/embed/VPA42ZnRo
15.5.4 - https://codesandbox.io/embed/JZ0mnE5oy

You'll need to have the console open to get the debugger statement.

In 15.6.1, the first change fires, but all subsequent changes do not fire. In 15.5.4, all changes fire.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 15.6.1 vs React 15.5.4; Chrome latest stable.

@gaearon
Copy link
Collaborator

gaearon commented Jun 16, 2017

cc @jquense

@jquense
Copy link
Contributor

jquense commented Jun 16, 2017

I'm not sure..this is a weird kinda case, The problem seems to be that if you don't control the the group the onChange event doesn't fire (but the value is correct)...The case of mixing defaultChecked and checked seems like an unsupported combo, either an input is controlled or not. cc @insin as well since he also noticed this.

@jquense
Copy link
Contributor

jquense commented Jun 16, 2017

Ah fairly certain I know what's happening. The checked value is tracked for each individual input, but when another radio is checked, there is no update to the last checked value so it remains true when you click back to it aect thinks it was already checked and so doesn't update it...I think this needs a simple call in updateCousins in ReactDOMInput

@jquense
Copy link
Contributor

jquense commented Jun 16, 2017

addendum this only happens with uncontrolled groups because the updateCousins method triggers an update on the component instance but since there is no value state changing (from the perspective of the instance) nothing changes.

I wish uncontrolled inputs didn't exist ;)

@MadaraUchiha
Copy link

MadaraUchiha commented Jun 18, 2017

Managed to reproduce this in a simpler test case, defaultChecked is not needed.

var Hello = React.createClass({
  render: function() {
    return <div>
    	<input type="radio" name="foo" onChange={() => console.log(1)} />
      <input type="radio" name="foo" onChange={() => console.log(2)} />
    </div>;
  }
});

ReactDOM.render(
  <Hello name="World" />,
  document.getElementById('container')
);

Changing between the radio buttons back and forth will only log 1 or 2 once. cc @Linkgoron

This seems like an actual bug where the state of React's view of the DOM differs from the actual DOM

See Fiddle for example: https://jsfiddle.net/aaqfzzjg/

@omninando
Copy link

omninando commented Jun 28, 2017

Managed to reproduce the problem comparing 15.3.1 vs 15.6.1, also without defaultChecked
15.3.1 - https://codepen.io/nandoacoelho/pen/XgVMxL
15.6.1 - https://codepen.io/nandoacoelho/pen/EXoWrY

@jquense
Copy link
Contributor

jquense commented Jun 28, 2017

To sum up the problem is uncontrolled radio/checkbox groups.

should probably update the issue title

@davidyu85
Copy link

Will this issue be fixed down the track?

Because this problem is affecting my web app and now I am stuck with the v15.4.1 to make my radio buttons work.

@MadaraUchiha
Copy link

The easy workaround is to manage the radio buttons with component state, instead of relying on the underlying DOM. But yes, this is a bit annoying.

@rhinoceraptor
Copy link

rhinoceraptor commented Jul 10, 2017

This is especially annoying for Redux/MobX users where DOM component state is never used. My app is 100% function components since it is using Redux.

@jquense
Copy link
Contributor

jquense commented Jul 10, 2017

It sure what you mean by that... An app that doesn't use DOM state won't have this problem since it would be using only controlled componenta

@rhinoceraptor
Copy link

You're right, I meant to say component state.

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

Successfully merging a pull request may close this issue.

7 participants