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

Warn when value and defaultValue are both specified #5819

Closed
jimfb opened this issue Jan 11, 2016 · 4 comments
Closed

Warn when value and defaultValue are both specified #5819

jimfb opened this issue Jan 11, 2016 · 4 comments

Comments

@jimfb
Copy link
Contributor

jimfb commented Jan 11, 2016

This should be illegal:

ReactDOM.render(
  <input value="foo" defaultValue="foo1" onChange={function(){}} />,
  document.getElementById('container')
);

Users need to decide if they are using controlled or uncontrolled inputs. We should warn if both properties are specified.

@quantizor
Copy link
Contributor

👍 I've run into this before and it can trip up new React developers.

@syranide
Copy link
Contributor

@jimfb I might be reading #5680 wrong, but doesn't that make it valid to use both? (Although, HTML defaultValue really should be separate from React defaultValue right?)

@jimfb
Copy link
Contributor Author

jimfb commented Jan 11, 2016

@syranide No, afaik, #5680 is trying to fix #4618 when using uncontrolled components. defaultValue is always meaningless for controlled components. #5680 does add a unit test that includes an element with both value and defaultValue, but I complained about that and hopefully it gets fixed before that gets merged (seeing that PR was the reason I created this issue).

@syranide
Copy link
Contributor

@jimfb Ah ok I see now... Hmm, I don't advocate it at all, but if that PR is meant to fix <input type="reset" />, wouldn't it make sense to support both value + defaultValue, defaultValue being what it resets to when you hit reset... rather than the value it already has.

Anyway, wrong issue to have this discussion in. 👍

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

No branches or pull requests

3 participants