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

Can't update defaultChecked/defaultValue. #4618

Closed
DylanPiercey opened this issue Aug 12, 2015 · 16 comments · Fixed by #6406
Closed

Can't update defaultChecked/defaultValue. #4618

DylanPiercey opened this issue Aug 12, 2015 · 16 comments · Fixed by #6406

Comments

@DylanPiercey
Copy link

When I rerender a component with "defaultChecked" or "defaultValue" react fails to update the "value" and "checked" attribute accordingly.

Before you say "use a controlled input" I would argue that this is a bug as it basically disallows uncontrolled forms with "reset" buttons. (It will always reset to whatever the initial default value was).

@DylanPiercey DylanPiercey changed the title Can't update default attributes. Can't update defaultChecked/defaultValue. Aug 12, 2015
@jimfb
Copy link
Contributor

jimfb commented Aug 12, 2015

You can specify a dynamic key on your uncontrolled input, and when you want to reset it, you can bump the key value (with the new defaultValue being empty string or being whatever "default/reset" value you want), which will effectively reset the component's internal state.

You could also break out of React and use plain javascript to manually reset the input's state (since you're using an uncontrolled input anyway, the component is effectively expecting triggers outside of React to modify it, so this doesn't break the component's mental model/contract).

Or you could just "use a controlled input" as you mentioned before, which is the correct solution IMHO.

Anyway, there are several options at your disposal. Your job as an engineer is to choose the one that's best for your use case.

@jimfb jimfb closed this as completed Aug 12, 2015
@DylanPiercey
Copy link
Author

@jimfb I understand that there are some hacks to achieve the desired result (which I am using) although I would expect that the "defaultValue" in my virtual dom accurately reflects the "defaultValue" in the real dom. It just seems odd for react to completely disregard any modifications to defaultValue.

@jimfb
Copy link
Contributor

jimfb commented Aug 13, 2015

defaultValue is, in some sense, the hack (for people who want a "fire and forget" solution to implementing their input tags). Think of it as an "initialValue" instead of "defaultValue" and it may make more sense to you. The point is that you get a single opportunity to specify the value at component creation time, and then the component takes over from that point forward.

React pushes you to top-down data flow for a reason. When you have siblings randomly influencing a node's state, it becomes very difficult to reason about the state of your application, and even harder to understand the ramifications of a given change.

If you need to be able to influence a component's state, then that state is no longer internal to the component (by definition). Modeling it as internal state is a violation of the abstraction because touching a component's internal state is a violation of the abstraction. If you need to control the component's state, you should probably be using a controlled component, IMO.

@DylanPiercey
Copy link
Author

@jimfb I understand your view, and what I want is a "fire and forget" form, which react seems to support.

It just seems odd that if uncontrolled forms are supported at all that you don't allow the browser to handle defaultValue and defaultChecked. All I am trying to do is reset an uncontrolled form and it seems impossible in react without hacks or rewriting it into a controlled form (Which I do not necessarily have control over since some of my dependencies use uncontrolled inputs and will not work with the native forms reset).

Even if my form was controlled because of this limitation it is unreliable to use the native "reset()" on any form because I cannot trust that react actually updated the default values for my form as my form is re-rendered.

So are you saying that react users should never rely on "reset" for forms that may have had their "initialValues" changed?

@jimfb
Copy link
Contributor

jimfb commented Aug 13, 2015

@DylanPiercey I'm saying that resetting the form currently "resets" the form elements to their initial values, which is the currently expected behavior (conceptually, the component is saving the initial property into state on first mount, and using that state variable from that point forward; you can't go back in time, so you can't change that initial value, because it was already sent). If you want more control over the state of your input components, I would recommend using a controlled input, which exists for the exact purpose of giving you the control.

You could always implement your own component which behaves like an uncontrolled input (does anything internally) and implements whatever semantics you'd like.

@sophiebits
Copy link
Collaborator

@DylanPiercey You can also grab the native DOM node and set .checked on it manually if you prefer. Honestly, I'm confused what API you're proposing though. How would React know when to reset the value and know when to keep it as what the user entered?

@DylanPiercey
Copy link
Author

@spicyj My current solution is to create my own inputs and do this:

Input = React.createClass({
  componentWillReceiveProps(nextProps) {
        if (
            nextProps.defaultValue !== this.props.defaultValue ||
            nextProps.defaultChecked !== this.props.defaultChecked
        ) {
          var input = React.findDOMNode(this);
          input.defaultValue = nextProps.defaultValue;
          input.defaultChecked = nextProps.defaultChecked;
        }
  },

  render() { return <input {...this.props}/> }
})

Basically it just keeps the defaultValue and defaultChecked in sync with the dom. Which only actually updates the input if it has never been touched by the user, or the form has reset. I guess this is just a little misleading because that is what the browser does by default when you modify "defaultValue".

I would propose something like this (as it seems like what we want and is unambiguous):

<input initialValue={ 1 } defaultValue={ 2 }/>

This could throw with both a defaultValue and initialValue, or initialValue could override defaultValue but only for the initial render.

@sophiebits
Copy link
Collaborator

Maybe we can make defaultValue update the DOM too. Few people use reset buttons so this hasn't come up.

@sophiebits sophiebits reopened this Aug 23, 2015
@DylanPiercey
Copy link
Author

@spicyj any further thoughts on this?

@sophiebits
Copy link
Collaborator

I'm happy to take a pull request that passes defaultValue through to the DOM. It looks like that is linked directly to the value attribute. Probably the best way to do this is to change our code so that value is treated as an attribute (using .setAttribute('value', x) instead of .value = x) in the property config which is equivalent to setting .defaultValue, then ReactDOMInput can manually set .value instead of going through DOMPropertyOperations. Open to other suggestions.

image

@CodeOfDavid
Copy link

@DylanPiercey I could not agree with you more

@leandro
Copy link

leandro commented Dec 4, 2015

👍 I definitely endorse this, as I'm also facing the same issue.

I simply want the ability to keep using my input as an uncontrolled input but with the ability to change the defaultValue/defaultChecked and reflect that change in the active DOM element.

@leandro
Copy link

leandro commented Dec 4, 2015

@DylanPiercey I just found a good solution for my case, so it might solve yours as well.
It turns out that in my case I was trying to render a form (component C) with some default values (in uncontrolled inputs) populated by a ajax request that is triggered by another component (a sibling -- lets call it component B). So whenever a new ajax request was triggered by user interaction on component B, it'll call beforeRequest callback provided by component A (the main component that has both B and C as children) as a prop to the component B (that actually does the request). This way in A scope, inside the callback, I render null for the form component (C) when a new ajax request is triggered. So between one form rendering an another, A renders null for C.

@sophiebits
Copy link
Collaborator

(Marking as good first bug – instructions are in my earlier comment; feel free to ask for clarification.)

@DylanPiercey
Copy link
Author

@leandro I mentioned my solution earlier which was simple a wrapper for all of my form inputs with a componentWillReceiveProps handler that checks for (and manually updates) defaultValue and defaultChecked this has worked okay, just sad that I can't use the regular elements.

@spicyj thanks for looking into this.

@darky
Copy link

darky commented Jan 12, 2016

👍

hayesgm added a commit to hayesgm/react that referenced this issue Jan 13, 2016
jimfb pushed a commit to jimfb/react that referenced this issue Apr 4, 2016
jimfb pushed a commit to jimfb/react that referenced this issue Apr 4, 2016
jimfb pushed a commit to jimfb/react that referenced this issue Apr 8, 2016
jimfb pushed a commit to jimfb/react that referenced this issue Apr 12, 2016
jimfb pushed a commit to jimfb/react that referenced this issue Apr 19, 2016
jimfb pushed a commit to jimfb/react that referenced this issue Apr 29, 2016
jimfb pushed a commit to jimfb/react that referenced this issue May 2, 2016
jimfb pushed a commit to jimfb/react that referenced this issue May 5, 2016
jimfb pushed a commit to jimfb/react that referenced this issue May 16, 2016
jimfb pushed a commit to jimfb/react that referenced this issue May 17, 2016
jimfb added a commit that referenced this issue May 25, 2016
* Have `defaultValue` reach DOM node for html input box for #4618

* Cleanup and bug fixes for merge.
zpao pushed a commit to zpao/react that referenced this issue Jun 8, 2016
…6406)

* Have `defaultValue` reach DOM node for html input box for facebook#4618

* Cleanup and bug fixes for merge.

(cherry picked from commit 4338c8d)
zpao pushed a commit that referenced this issue Jun 14, 2016
* Have `defaultValue` reach DOM node for html input box for #4618

* Cleanup and bug fixes for merge.

(cherry picked from commit 4338c8d)
gaearon added a commit that referenced this issue Aug 31, 2018
jetoneza pushed a commit to jetoneza/react that referenced this issue Jan 23, 2019
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.

6 participants