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

Document a way to "update" the defaultValue #3357

Closed
TheLudd opened this issue Mar 10, 2015 · 11 comments
Closed

Document a way to "update" the defaultValue #3357

TheLudd opened this issue Mar 10, 2015 · 11 comments

Comments

@TheLudd
Copy link

TheLudd commented Mar 10, 2015

There seems to be a lot of misunderstanding in the react community about how defaultValue works. It is often expected that if the defaultValue of an element is changed then that should be reflected in the UI. This is not the case since defaultValue is only set when the component is first rendered.

When people complain about this (#1484, #1663, #2764) it is usually suggested that they use value instead of defaultValue but this is often seen as unpractical in a form were there are many input fields since all fields now need an onChange listener to be editable. (This is not actually true, you can put an onChange listener on a wrapping element but this generates a lot of warnings #1118, #3070).

I feel that none of the above solutions are practical, but an easy solution is to set a key attribute on a wrapping element. If the form contains elements with defaultValue fields that are changed along with the key, the change will be reflected in the UI.

I think this is a good solution for a common UI case where you have a list of entities you want to be able to edit. If you click one of them a form shows up with the entity value populated and if you click on another element you want the form to be updated with that entity's data.

Perhaps it could be documented in order to avoid future confusion. Or is there an even better solution?

@syranide
Copy link
Contributor

When people complain about this (#1484, #1663, #2764) it is usually suggested that they use value instead of defaultValue but this is often seen as unpractical in a form were there are many input fields since all fields now need an onChange listener to be editable.

They can all share the same onChange-listener so I don't see the issue, but yeah some people don't like to do that for some reason.

Or is there an even better solution?

There are two approaches for updating uncontrolled inputs, neither is universally right:

  1. Assign a ref and call this.refs.myInput.getDOMNode().value = .... This "should" be used when the underlying value has changed and the input should be updated to reflect it.
  2. Assign a key (mostly preferable to do it further up on a shared parent). This "should" be used when the input has changed "source", i.e. it's now referring to another unrelated value, like an input on a different tab. Assigning a key will completely destroy the previous input, along with focus, etc.

The second kind of applies to controlled components as well, but it's rarely something that really matters.

It may be more obvious if you think of it in-terms of traditional DOM interaction, either you update the value of an existing input. Or e.g. when switching tabs you destroy the DOM and render it again and thus the newly rendered inputs show the current value.

@TheLudd
Copy link
Author

TheLudd commented Mar 10, 2015

They can all share the same onChange-listener so I don't see the issue, but yeah some people don't like to do that for some reason.

You mean setting onChange={ this.myListener } on each input element? No I certainly don't prefer that as it is duplication of code and really unnecessary.

It is the second case that I believe people want to implement the most when they run into this issue. But I haven't seen this solution presented anywhere. Along with the risks such as focus loss.

@syranide
Copy link
Contributor

onChange={this.mySharedListener.bind('mykey')}

@TheLudd
Copy link
Author

TheLudd commented Mar 10, 2015

What I want to do is

<form onChange={myFormListener} >
   { arbitrary number of input fields letting their events bubble to the listener on the form }
</form>

Through this I specify the listener once and not once per input field.

@joshbedo
Copy link

@TheLudd You should be able to do exactly what you mentioned above. When the form changes just use ev.target to figure out which input has changed within the input form. Form Event Listeners

@TheLudd
Copy link
Author

TheLudd commented Mar 10, 2015

@joshbedo Yes I know, but it generates warnings which makes it a no go for me (and many others who have commented on this here on github). I always strive to reduce the number of warnings to 0

This is not actually true, you can put an onChange listener on a wrapping element but this generates a lot of warnings #1118, #3070.

@hufeng
Copy link

hufeng commented Mar 12, 2015

yes, i am also want to bind onChange on form, not each form input field. but react give many warnings.

@rohan-deshpande
Copy link

How about a method like forceUpdate forcing a complete re render?

@KeKs0r
Copy link

KeKs0r commented Jun 9, 2015

Struggling with the same issue, and thanks to @TheLudd I now at least know how to even remotely solve this (I added a dynamic key to my form, which is not the prettiest solution either).

How did u get around the issue with the lost focus?

@jimfb
Copy link
Contributor

jimfb commented Jan 11, 2016

I think the official answer is to use controlled components. There are other techniques, like using keys or the imperative DOM API, but these are escape hatches rather than officially endorsed solutions (documenting them beyond what's in the github issues would give them too much legitimacy).

The confusion about defaultValue was mostly solved in: #4379, which gives our official "reacty" answer. There hasn't been much confusion/pressure since that PR merged, so I think we can close this issue.

@jimfb jimfb closed this as completed Jan 11, 2016
@proehlen
Copy link

proehlen commented Feb 1, 2016

Controlled components is not the same as having a reactive defaultValue. The former never lets you change it. The latter is necessary when it is necessary to change the default value for some reason but allow the user to enter their own value.

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

No branches or pull requests

8 participants