When the value property of a textarea changes to null or undefined from a non blank value, the text displayed in the DOM does not change. Here is a fiddle that captures the issue - notice that behavior is different for the text input.
I believe it's input
that is misbehaving here, not textarea
. Setting value
to null
makes it uncontrolled and it should make no attempt to change the value.
Here's the case I'm thinking of: Imagine a series of state transitions (perhaps instead of a single button the form would have some sort of selector that loads a series of objects from a data source). If one of those transitions sets a property to null, you end up with a value from the last object in the textarea. Doesn't seem right.
I can see though what you're saying. I might be taking the concept of two way binding a little too literally - I'm currently working around it by doing something like this:
http://jsfiddle.net/f1v1fac1/2/
...which isn't a big deal.
@syranide does setting to null make it uncontrolled? That doesn't sound right to me, but I haven't looked at the linkedstatemixin in a long time. I also would expect the behavior of the input here, not the textarea.
@zpao http://jsfiddle.net/91khmysc/ null
is treated as (mostly) identical to undefined
(or not set) through-out React I think?
Also yeah I agree, the more I think about it more it makes sense to "reset" it when becoming uncontrolled, it's the "most deterministic" one and also the one easiest to reason about.
This one was driving me nuts too until I finally got down to the bottom of things. Here's a simpler example that doesn't use valueLink
.
http://jsfiddle.net/wn7ymsa4/3/
Pretty sure if a value/valueLink/checked/checkedLink
property was given the component should be controlled, regardless of the value assigned.
The same problem also occurs for a <select>
value. I've extended the jsfiddle to show the select-problem, too:
http://jsfiddle.net/27bqLvLL/1/
Okay, I think my pull request above #3041 should fix the issue and have correct logic (so this one should be preferred over the pull request #2534).
If I get the heads up, I have a similar fix for the same problem for <select>
components.
I still think react should be checking for the presence of a value
key, not the value itself. If you want the component to be uncontrolled, don't pass a value prop.
// controlled
<textarea value={this.props.text} />
// uncontrolled
<textarea />
Removes any possible confusion since you no longer need to know all possible values of this.props.text
to know if the textarea is controlled or not.
I don't disagree, but I think your line of thinking is a bigger change (that would also affect the documentation). So I'd suggest to go with the pull request #3041 for now.
But for the overall picture, I think you're right. There should be a clearer distinction between controlled/uncontrolled behavior, that avoids edge cases like these:
http://facebook.github.io/react/tips/controlled-input-null-value.html
@rymohr While it makes sense in a way, it breaks with how all other props work and I feel like there's something inherently wrong about <textarea value={undefined} />
being a controlled component as well. While I don't have this on authority, I'm pretty sure that JS defines undefined
as meaning to be literally undefined, treating it as anything else breaks expectations.
True, but this is about developer intention and "falling into the pit of success" as you guys like to say. It happens with null
values too, not just undefined
.
Nobody passes a value
prop to a component they want to be uncontrolled. They use defaultValue
and placeholder
.
Switching to a key-presence check would simplify the underlying code and make controlled vs uncontrolled components easier to explain. It's easy to misinterpret the current documentation:
An with
value
set is a controlled component.
In the examples above, value
_is_ set, it just happens to be set to null
.
I believe it's input that is misbehaving here, not textarea @syranide
It should also be revealing that nobody's complained about the current behavior of input
-- it's the textarea
behavior people find surprising (regardless of which is technically misbehaving by react expectations).
cc @yungsters (for here and #3041) - I know it's been a while but you may remember some of your early thoughts around controlled components.
I'm not sold that returning to defaultValue
is the right call. But in general this whole thing is a bit of a grey area. We should be consistent, regardless of which route we take.
@zpao Returning to defaultValue
is the only that makes sense to me, it would be the equivalent of treating them as separate modes/components, i.e. it resets when you switch between them. It was you who convinced me first :P
To me it becomes quite obvious when you think of React as being inherently kind of stateless, for a certain set of props you should always get the same output. So rendering <input defaultValue="test" />
sometimes rendering to "test" or ""/last value (without user interaction) depending on whether the input was previously controlled makes no sense to me, there's no transition between controlled/uncontrolled that really makes practical sense. They're different.
1. <input defaultValue="foo" />
=> "foo"
2. <input value="bar" />
=> "bar"
3. <input defaultValue="foobar" />
=> "foobar"
4. <input defaultValue="barfoo" />
=> "foobar"
Nothing else really makes sense to me in the context of React.
But if the user typed something, we leave that value in the DOM on subsequent renders. You could think of React as stateless but the DOM isn't and we respect that for uncontrolled components.
(same render fn I assume)
1. <input defaultValue="foo" />
=> "foo"
2. user types "bar"
=> "bar"
3. <input defaultValue="foobar" />
=> "bar"
If we treat changing the value
prop like we treated a user manually changing the value, then we wouldn't return the defaultValue
. I'm not saying that's the right thing, just that we have precedent.
@zpao Yeah ofc. I improved my post a bit after I posted it. To me it comes down to the fact that transitioning from controlled to uncontrolled doesn't really make sense to me, you pick one and stick to it. You're not supposed to use controlled as a read-only state for your uncontrolled input. So if there's a controlled to uncontrolled transition, it's really a transition from <ControlledInput>
to <UncontrolledInput>
so it should reset... i.e. you forgot to give it a proper key.
EDIT: Or is there a use-case for transitioning from controlled to uncontrolled? Then it could make sense to keep the value, but I can't think of any...
Also; it's easy to reproduce the behavior of the value sticking by simply updating defaultValue
appropriately. But if it always keeps the last value then the only way to get it back is to manually detect the transition and update the value of the DOM node.
@zpao Hmm, actually, let's rephrase it like this:
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. (something happens)
4. setState({value: 'bar'});
5. <input value={this.state.value} />
6. <input defaultValue={this.state.value} />
=> "bar"
That was easy, now let's consider this instead:
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. (something happens)
4. setState({value: 'bar'});
5. (noop or batched updates)
6. <input defaultValue={this.state.value} />
=> ???
I'm quite convinced now that the correct answer at step 6 is "bar" (i.e. it should reset to defaultValue
). If it was "foo" you would be looking at an old irrelevant value, whether React re-rendered in-between (consider rAF-batching or whatever) should be irrelevant to the visual state shown to the user.
@zpao I agree that the approach should be consistent.
The most consistent approach for value
would be the one mentioned by @rymohr which is basically just one simple rule:
When a value is present, it is a controlled component. (... even if that value happens to be null).
Imagine we have a controlled component with a non-null value that gets changed to null
. Now there are 2 interpretations:
defaultValue
or keep the previous content? What would be consistent?null
. I feel that this is almost a _must-have_ behavior, because otherwise how would you clear the component?What is the reasoning behind the current definition/behavior? If there aren't any good reasons besides "that's just the way it is defined", then it would probably be better to change the definition.
@martinstein You still have transitions between controlled and uncontrolled inputs regardless of whether value={null}
is considered controlled or not.
EDIT: I think turning value={null}
into a controlled component is rather counter-intuitive, the solution should be to make uncontrolled and controlled inputs separate components, not using magic properties/values. Also, note that you can easily create your own wrapper to enable the behavior _you_ prefer.
@syranide You are right, there are still transitions between those two types. But there would be fewer ambiguities/inconsistencies.
Can you explain why you find value={null} equals controlled
so counter-intuitive? I feel that if that behavior was intuitive, then we wouldn't need an extra page explaining this phenomenon:
http://facebook.github.io/react/tips/controlled-input-null-value.html
And the 'rule-set' would just be simpler.
Currently the rule is:
When a value is given, it is a controlled component. Except for a value of `null`, then it's uncontrolled.
The simpler rule would be:
When a value is given, it is a controlled component. Period.
Generally, the simpler approach is the better and more intuitive one, right?
@martinstein It's not so much that I find <input value={null} />
to be controlled counter-intuitive, but that it breaks basic expectations of what a null
value means and that is counter-intuitive. null
means default/no value if you ask me, you would expect foo(null) === foo()
to hold, just like I would argue that foo({enabled: null}) === foo({})
should hold and by extension <input value={null} /> === <input />
. null
is also commonly used to optimize hidden classes where it's generally expected to have the same behavior as if not having been specified unless for merging objects.
I don't think it breaks basic expectations. Most people would expect that <input value={null}>
means exactly that: "We have an input with an empty value". If that were supposed to mean: "We have an uncontrolled input", then why specify the value at all? And (sorry for the repetition) if that were intuitive, then that extra page in the docs would not be necessary.
From a language perspective, JavaScript specifically treats null
different than not being defined at all:
var something = {test: null};
something.hasOwnProperty('test') // is true
That, for me is the expectation of how null
should be evaluated. So foo({enabled: null}) === foo({})
should most definitely not hold!
I agree that it's sometimes used as a short-cut to optimize code but in this case it makes the code more complicated (my pull-request would have been simpler without the special treatment of null
).
I think we've collectively beaten this one to death at this point. Any new eyes from the core team that can take a look at it and give a fresh take?
I don't think that setting value
to null
or undefined
should reset the DOM to defaultValue
. If this behavior is desired, the composing component should continue setting value
. Otherwise, setting value
to an uncontrolled state should do just that — leave the DOM as-is. Letting component authors rely on conditionally setting value
in order to reset defaultValue
is pretty confusing, and I would not want anyone relying on this behavior even if we were consistent about it.
As for whether or not null
should control the DOM, if we allow null
to mean that a component's value is controlled, what is that value? For <input>
and <textarea>
, you may argue it should be an empty string. But what about a <select>
with <option value="" />
and <option value="null" />
? This ambiguity make me think that null
should not control a prop value. Besides, it is very reasonable for component authors who want a controlled value of the empty string to set value=""
.
We should fix <input type="text" />
.
@yungsters But if we take my example above (copied below), I think the only thing that make sense is to "get the latest value and set it before becoming uncontrolled" which is equivalent to a reset.
The example from above, consider this scenario:
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. (something happens)
4. setState({value: 'bar'});
5. <input value={this.state.value} />
6. <input defaultValue={this.state.value} />
=> "bar"
That's obvious, now let's consider this instead where the new state is not rendered due to batching (or w/e):
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. (something happens)
4. setState({value: 'bar'});
5. (noop or batched updates)
6. <input defaultValue={this.state.value} />
=> ???
I would argue quite strongly that the value should still be "bar", whether or not the component actually performed intermediate re-renders should be irrelevant, especially as it's affected by the batching strategy. If we _reset_ when becoming uncontrolled then it's always consistent.
That said however, I don't think going from controlled to uncontrolled is something you should do (the best solution would be if it wasn't at all possible if you ask me), we should simply make the best of it. But _if_ you do, this is the only one that makes sense to me in the context of how React is meant to work, this is deterministic in a way that "keeping the last value" simply isn't.
Think about a simple presentation editor that allows you to edit the text for a slide. Most people are going to model the slides like this:
slides = [
{id: 1, text: "one"},
{id: 2, text: undefined}
]
The form has a "controlled" <textarea>
bound to this.state.slide.text
. If you move from a slide 1 to slide 2, you'll still see the old text. Letting this nuance of the way react handles controlled components leak into the data model just doesn't feel right.
I don't think the <select>
issues you mention are actually a problem, especially since you control the options for the select. Both null
and undefined
should be treated as ""
-- if you wanted anything else you need to pass it as a string (eg "null"
).
@rymohr
The form has a "controlled"
Huh? If it's controlled then it by definition always shows the correct value.
@syranide why not prevent components from going from controlled to uncontrolled? Makes sense to me. If you wanted to do that you could just use something like
render: function() {
return this.state.controlled ? <input value={this.state.value} /> : <input />;
}
Though I can't think of any good reason to do that.
@rymohr It is the consumer of slides
that is responsible for translating a configuration of undefined
text to an empty string when rendering a React component.
@syranide > Huh? If it's controlled then it by definition always shows the correct value.
Isn't that the whole point of this discussion? We're arguing over what defines a controlled <textarea>
. See my first comment in this thread: https://github.com/facebook/react/issues/2533#issuecomment-63731720
@syranide But what about when no defaultValue
is supplied?
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. (something happens)
4. setState({value: 'bar'});
5. (noop or batched updates)
6. <input /> // No defaultValue
=> ???
In this case, I would argue that the input should have a value of bar
and not be empty.
@yungsters by that logic it's also the developer's responsibility to sanitize all input but we still have dangerouslySetInnerHTML={{__html: ...}}
. The point is about aligning with developer expectations and intentions and preventing them from shooting themselves in the foot.
The 6-step argument you guys are having would be moot if you simply said passing a value
prop (_any_ value) creates a controlled component.
@yungsters I don't think there's anything ultimately right or wrong here and there's no use-case that _really_ makes sense.
I see two options to this ambiguity:
Keep the last value
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. setState({value: 'bar'});
4. <input value={this.state.value} />
=> "bar"
5. setState({value: 'foobar'});
6. <input defaultValue={this.state.value} />
=> "bar"
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. setState({value: 'bar'});
4. (batched update)
5. setState({value: 'foobar'});
6. <input defaultValue={this.state.value} />
=> "foo"
Even though it may make intuitive sense I don't see when you would ever want that behavior. The final value depends on the batching strategy and whether or not the operations were batched. Regardless, if the controlled to uncontrolled transition should make _any_ sense there has to be a relation between the two (otherwise you should key it properly) and I would argue that in this case you are always seeing an old value. Consider:
render: function() {
return (
this.state.value !== 'magic' ?
<input value={this.state.value} onChange={...} /> :
<input />
}
If you type magic
then it will be uncontrolled and if we weren't relying on the stateful DOM ideosyncrasies the value of the input would actually be magi
, not magic
(can be reproduced other ways as well, feel free to replace magic
with wizard
in the change handler to the highlight the issue).
I fail to see when this ever makes practical sense, not saying a use-case doesn't exist, but I don't see it. It just seems arbitrary and in some sense condones controlled to uncontrolled transitions as being legitimate, which I'm not sure if I agree with in the context of React and batching strategies, etc, etc.
Reset to default value
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. setState({value: 'bar'});
4. <input defaultValue={this.state.value} />
=> "bar"
5. setState({value: 'foobar'});
6. <input defaultValue={this.state.value} />
=> "bar"
1. setState({value: 'foo'});
2. <input value={this.state.value} />
=> "foo"
3. setState({value: 'bar'});
4. (batched update)
5. setState({value: 'foobar'});
6. <input defaultValue={this.state.value} />
=> "foobar"
Again we're seeing two different values, but the reason for this is simply the nature of uncontrolled components. But more importantly neither is inherently wrong in my opinion, the value shown is simply subject to the inherent shortcomings of uncontrolled components. Consider this example again:
render: function() {
return (
this.state.value !== 'magic' ?
<input value={this.state.value} onChange={...} /> :
<input />
}
When you type magic
it resets and becomes empty, don't ask me why you would want this. But we're showing a deterministic state, not magi
. If we also take the alternative example you have:
render: function() {
return (
this.state.value !== 'magic' ?
<input value={this.state.value} onChange={...} /> :
<input defaultValue={this.state.value} />
}
When you type magic
it resets and becomes magic
as you would expect. Again, don't ask me why you would want to do this, but we're again showing a deterministic state.
Now include the fact that resetting like this is also near identical to proper keying (except for maintaining focus), that seems like another benefical side-effect. We're treating controlled as separate and incompatible with uncontrolled just as they should be (but still maintains a practically reliable behavior).
To me this is the obvious choice all things considered...
Ultimately (from an academic perspective), I wouldn't mind <input />
being controlled only, if you want the uncontrolled behavior you'll have to use a wrapper or a generic makeUncontrolled(React.DOM.input)
if it's reasonable to agree that value
and onChange
is the de-facto interface for all controlled components.
@rymohr What you're describing is how getDefaultProps
used to work. If any value was provided (including null
or undefined
), we would use it over the default value.
However, this was a pain for building components that sometimes wanted the default value and other times wanted a specific value. For example:
var Button = React.createClass({
getDefaultProps: function() {
return {type: 'submit'};
},
// ...
});
var FancyButton = React.createClass({
render: function() {
// This sucks.
if (this.props.hasOwnProperty('type')) {
return <Button type={this.props.type} />;
} else {
return <Button />;
}
}
});
We're treating controlled as separate and incompatible with uncontrolled just as they should be.
Hmm... actually, I like this a lot. @zpao, what do you think? I'm willing to go with this as a reason for "resetting values" when changing between controlled and uncontrolled.
@yungsters I think you misunderstood me. Underscore has established an accepted precedent for merging default values, where defaults always overwrite undefined values. If you handled them any other way it would surprise developers.
All I'm saying is that by passing a value
key, the developer is giving you a massive hint that they want the component to be controlled and you're ignoring it.
I suggest we bring this discussion back to the original issue posted by @rszewczyk:
When the value property of a textarea changes to null or undefined from a non blank value, the text displayed in the DOM does not change. Here is a fiddle that captures the issue - notice that behavior is different for the text input. http://jsfiddle.net/f1v1fac1/
We're talking about current behavior not lining up with expected behavior. You can only argue that the textarea holding onto "bar" is correct from an internal architecture standpoint. As an end user it's completely counter intuitive.
Reset yes, but while you're at it why not simplify the entire controlled vs uncontrolled thing as well?
@rymohr I agree with you 100%. I stumbled into this issue because I was dumbfounded by how textarea
behaves so illogically.
React team: This behavior is _totally_ unexpected. PLEASE make textarea
remain controlled when I set the value to undefined
.
To any other peons out there wondering why this doesn't work, here's my ghetto fix:
<textarea value={myVariable || ''} />
Reset yes, but while you're at it why not simplify the entire controlled vs uncontrolled thing as well?
What kind of simplification are you looking for besides resetting the mounted value when switching in/out of being controlled?
@yungsters please reread my comments if that part's still unclear.
I just want to chime in here and mention that, at the least this behavior should consistent across all Input fields and textarea. Why is it that textarea is treated differently?
@uptownhr As far as I know the behavior is the same across input/textarea. Am I misunderstanding?
@spicyj see https://github.com/facebook/react/issues/2533#issuecomment-63731720 above.
@spicyj here's a better example: http://jsfiddle.net/wn7ymsa4/12/
Given this issue has been open for around 10 months now, and people been discussing it since. What is the verdict? Seems pretty straight forward to me, that all the input fields at the least needs to adhere to one standard on how it treats null || undefined
.
How can we move forward with a fix?
I propose changing textarea to adhere to how the other input fields behave and removes the text when the value turns to null || undefined
I think this is the core issue here.
First thing we need to decide is what the ideal/correct behavior is, since no matter what we do here it is a behavioral change.
One option is that null clears the input element and makes it uncontrolled. Another is that it makes the input element uncontrolled without resetting the value. Another is to return it to the defaultValue. Another is that we keep the current semantics (or forestall the change in semantics - for now, text areas and input fields behave differently) which avoids introducing subtle changes in behavior that will break people's apps.
Once we decide the desired behavior, we can move forward with a fix. cc @spicyj @sebmarkbage Thoughts?
Just wanted to state, that I have just stumbled upon this issue. First we had an input field for texts, then we made it a textarea and now it behaves differently. I would personally expect them to work in the same way ...
@davidreher IMHO that's a bug in your code, you should not be relying on this behavior. Even though inputs behave as you expect, you're not using them as intended and I'm sure there are some unintended side-effects as a result for certain edge-cases. EDIT I should add; that is if you're using nulls instead of empty strings, intentionally switching between controlled and uncontrolled behavior is fine and that's what should be fixed.
@syranide thx, we will fix this to use empty strings instead of null values.
I think this horse died months ago but I still think things would be easier to reason about if the controlled/uncontrolled test was simply a key test on props. If you passed a value
prop the component would be controlled, regardless of what value was actually given (null
, undefined
, and ""
would all result in a controlled blank input). Seems to fit react's "fall into the pit of success" motto much better than "IMHO that's a bug in your code."
I'm curious though: what's the use case for switching between controlled and uncontrolled states for the same input? I've never needed that behavior in practice.
@rymohr Our intention was that a prop value of undefined
is always equivalent to not providing it. This is how defaultProps, etc. works. In most cases, null
is what people end up passing and so we could make that mean the empty string, as discussed above.
@spicyj that's a good practice to follow in general. Controlled components feel like an exception though. Sticking to that approach leaves you with a bunch of value || ""
calls that are easy to forget, and you're completely ignoring the developer's intention to provide a value for the input (signaled by passing a value
prop).
I think I mentioned this earlier in the thread, but the red flag to me is that nobody complains or is surprised about the text input behavior (null/undefined clears controlled component). It's always the textarea behavior that surprises them.
Are there any good examples where value
is being passed and the intention is not to create a controlled component?
@spicyj What we're really concerned about in the end is unintentionally switching controlled/uncontrolled right? Perhaps something we should warn for (once) too?
I'm going to point this out again, if null || undefined
is supposed to be treated as uncontrolled, why is it that other input types other than textarea do not follow the same behavior.
type=text, clears when turned to null||undefined
type=textarea, becomes uncontrolled but the last given value remains.
I'm tempted to agree with @syranide and say we should warn (once) when a component switches from controlled to uncontrolled. Valid use cases are exceedingly rare, and I'm thinking it's almost always a mistake.
Warning in such cases also gives us an upgrade path to change the behavior of passing undefined
to a controlled textarea, since it is sounding like the best behavior is to reset to the empty string.
@syranide @jimfb :+1: sounds like a solid plan.
Valid use cases are exceedingly rare, and I'm thinking it's almost always a mistake.
@jimfb Expanding on that, I would go as far as saying it's either a mistake or bad practice. First; if controlled/uncontrolled where separate components (as they really should be IMHO), it would be impossible to switch like this. Second; it's trivial to implement uncontrolled behavior using a controlled input, so if what you really want is some non-standard behavior you're best off implementing your own component on-top of controlled inputs that implements whatever behavior you want, not switching between uncontrolled/controlled.
My point being; yes, there may be _valid_ use-cases, but anyone being serious about their code can and should always avoid the warning as above. So I personally don't see any negative sides to having this warning.
value || ''
is not a good solution because sometimes you have number 0 which is controlled too and you will lose this value
0 || '' => ''
here is acceptable fix:
function fixControlledValue(value) {
if (typeof value === 'undefined' || value === null) {
return '';
}
return value;
}
Hi everyone,
this issue is still open so I guess nothing had been settled?
Even though IMO input and textarea should work the same (but I dont have a deep knowledge of it so I dont get in the conversation) I implemented what you guys wrote (btw good link
https://github.com/erikras/redux-form/issues/394)
Nevertheless, is this the final call? Or are we waiting for a confirmation this is the target implementation? (just to know if we will have to change our code anytime soon)
Cheers
@iboxgithub You should not rely on this behavior and I believe 0.15 will issue a warning if you do.
You mean 15.0
On Feb 23, 2016, at 2:24 PM, Andreas Svensson notifications@github.com wrote:
@iboxgithub You should not rely on this behavior and I believe 0.15 will issue a warning if you do.
—
Reply to this email directly or view it on GitHub.
Thanks for your feedback --> so what is THE best practice :)
@iboxgithub An input should be either uncontrolled (value always undef/null) or controlled (value is a string, so it should be an empty string rather than null) for its entire lifetime.
I don't think there's anything actionable here, so I'm going to close this out. We still warn when trying to reset a controlled input using null
, and still recommend @syranide's advice:
An input should be either uncontrolled (value always undef/null) or controlled (value is a string, so it should be an empty string rather than null) for its entire lifetime.
Most helpful comment
@iboxgithub An input should be either uncontrolled (value always undef/null) or controlled (value is a string, so it should be an empty string rather than null) for its entire lifetime.