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

Treat value={null} as empty string #11417

Open
IndifferentDisdain opened this issue Oct 31, 2017 · 17 comments
Open

Treat value={null} as empty string #11417

IndifferentDisdain opened this issue Oct 31, 2017 · 17 comments

Comments

@IndifferentDisdain
Copy link

IndifferentDisdain commented Oct 31, 2017

Per @gaearon's request, I'm opening up a new issue based on #5013 (comment).

Currently, if you create an input like <input value={null} onChange={this.handleChange} />, the null value is a flag for React to treat this as an uncontrolled input, and a console warning is generated. However, this is often a valid condition. For example, when creating a new object (initialized w/ default values from the server then passed to the component as props) in a form that requires address, Address Line 2 is often optional. As such, passing null as value to this controlled component is a very reasonable thing to do.

One can do a workaround, i.e. <input value={foo || ''} onChange={this.handleChange} />, but this is an error-prone approach and quite awkward.

Per issue referenced above, the React team has planned on treating null as an empty string, but that hasn't yet occurred. I'd like to propose tackling this problem in the near future.

Please let me know if I can help further.

@gaearon
Copy link
Collaborator

gaearon commented Oct 31, 2017

@sophiebits Is this interpretation correct? Was there any reason we didn't do this in 16?

@sophiebits
Copy link
Collaborator

I don't remember. I don't think it's obvious which is better: I could imagine a case where you expect value={null} defaultValue="foo" renders an input with an (uncontrolled) value of foo. Treating null as controlled empty string would be surprising then. Also, all of our host/DOM components treat null and undefined identically right now, I believe. Don't know if we want to deviate from that.

@IndifferentDisdain
Copy link
Author

Not to scope creep this thread, but wouldn't you generally use value for controlled inputs and defaultValue for uncontrolled? I guess I don't see why someone would use both attributes on the same element.

@paulirwin
Copy link

Allowing value={foo} where foo is null without warning may not be a very strong argument in the case of <input type="text" value={foo} />, but it does make more sense with input type number where using empty string to represent a missing number value feels counter-intuitive: <input type="number" value={typeof foo === "number" ? foo : ""} />

(Aside: it would also be nice to be able to bind Date objects to the value of <input type="date" />, and I only bring it up because that would make this case even stronger.)

@nickgcattaneo
Copy link

I face this issue often when building controlled input's who rely on a record from state slice (such as an Immutable.Record via redux). I skip using something like defaultValue, because as my input changes, I dispatch various actions to update the record (which other components listen to, etc for various selections, calcs, etc) and ultimately the controlled value is 100% dependent on the value prop passed itself. I usually just set in my record the default value as '', but it would be great to use null since I can tell (without a pristine flag) if the field is pristine or simply been edited based on this concept.

As noted here, defaultValue only satisfies the initial life cycle & muddying the two seems like an extraneous step:

https://reactjs.org/docs/uncontrolled-components.html

@rdsedmundo
Copy link

rdsedmundo commented Apr 25, 2018

This problem happens to me very often when I'm using redux-form, with the same situation that you described in the issue's title.

I have to change initial values to either undefined (making them uncontrolled them) or to empty string even when situations that they aren't strings. It works because at the beginning, on an input everything is a string, but it's counterintuitive.

I'd like so much to be able to use null as initial values because it's good to be able to distinguish when something is really undefined or it's defined but it has no value (null). Like this:

const someObject = {
  a: null,
};

someObject.a; // existent property, defined as null
someObject.b; // inexistent property, returns undefined

/* versus */

const someObject = {
  a: undefined,
};

someObject.a; // existent property, defined as undefined
someObject.b; // inexistent property, but returns undefined as above

@RyanNerd
Copy link

RyanNerd commented May 6, 2018

I'm new to React and still learning the ropes. It surprised me that controlled components do not allow nulls. Coming from a SQL background null is a fundamental concept. In a small database having the often unused address2 field store an empty string value isn't a problem, but with a table with millions of records -- each record occupies drive space of millions of single empty string bytes that really should be null thus increasing drive space and degrading performance. Another example are averages, sums, and other aggregate calculations null are not part of the calculation so a query that calculates the AVG() is accurate because 0 !== null.

To work-around this shortcoming in React I've created shadow property flags for any fields that can be null -- with logic at commit time to use null for 'empty' fields that are marked in my shadow properties as nullable. The point is that I shouldn't have to do this. Null as a "value" has meaning in many contexts especially if your back-end data-store supports nulls and benefits greatly from their use.

@ernestopye
Copy link

ernestopye commented May 10, 2018

Running into this myself. Similar scenarios as described above (API returns object with null property values). I'm looking to go with the solution @RyanNerd is proposing, but it doesn't feel right. Alternatively, I can change my input to value={value || ''} but then change my setState call so that it converts '' back to null by default. It's not ideal, but for my scenario it may be enough.

To avoid breaking existing projects, would it be possible perhaps to have an attribute that allowed you to specify that the input is controlled, regardless of the value?

<input forceControlled type="text" value={someNullValue} onChange={this.myChangeHandler} />

I'm new to React and know very little about its internals. Just throwing something out there.

@RyanNerd
Copy link

I understand @sophiebits concern with how changing React to suddenly start treating nulls as a legitimate "value" for controlled components may have an impact on React.
I like @ernestopye 's suggestion of using a property/attribute in the control such as allowNullValue=true that would let developers opt-in controls that support this feature, with perhaps a future version of React not requiring this at all.

@jedwards1211
Copy link
Contributor

jedwards1211 commented Aug 3, 2018

Another case where this is really annoying is when using Material UI's Select component, which renders an <input> in non-native mode. When I want nothing selected, what immediately pops into my head is value={null}, but instead I have to do the very non-intuitive value="".

The time wasted on petty little issues like this can really add up. Sometimes I find it more annoying than major bugs.

I like @ernestopye's suggestion, I would just say controlled is the ideal name for the prop, rather than forceControlled or allowNullValue, and when the input is marked controlled, it should even allow value={undefined}.

Personally, I wish that that the uncontrolled case were opt-in only via an uncontrolled property.

@gaearon
Copy link
Collaborator

gaearon commented Aug 15, 2018

My takeaway so far is that changing the semantics of null as suggested in some comments in this thread will not reduce the confusion.

If we do it, I think other people will come with complaints — just as valid as the ones in this thread — about null "unexpectedly" being treated differently from undefined. It's more obvious that treating null as uncontrolled today is confusing because that's what React does today. But it doesn't mean that changing the behavior will make it any less confusing. And the cost to changing the behavior is very high so it's not clear to me that we should do it.

It's understandably frustrating. I agree that the core of this issue is that null is ambiguous and it's not clear what it means. Maybe one of you would like to submit an RFC for the controlled prop or an alternative? https://github.com/reactjs/rfcs

@styfle
Copy link
Contributor

styfle commented Aug 15, 2018

I don't use uncontrolled inputs so I'm struggling to see why this can't be changed.

In a comment above, @sophiebits mentioned that someone might use value={null} defaultValue="foo" which I think is safe to say that this is an uncontrolled input because defaultValue is defined. Afterall, the defaultValue property is only mentioned on the "Uncontrolled Components" documentation.

@gaearon What other scenarios do you think will cause people to complain if this gets changed? Do you have use cases that would break that you see in the Facebook code-base somewhere?

@RyanNerd
Copy link

RyanNerd commented Nov 4, 2018

@gaearon is correct when he said:

the core of this issue is that null is ambiguous and it's not clear what it means

The core of this problem isn't with React. It's with the insane JavaScript language itself:

!!null; // -> false
null == false; // -> false
typeof []; // -> 'object'
typeof null; // -> 'object'

// however
null instanceof Object; // false
null > 0; // false
null == 0; // false
null >= 0; // true

Given that JavaScript is demonstrably insane and inconsistent with how nulls are treated what are we to do? This appears to me to be a matter of type safety. With controlled components React would need to be told what type to expect if the null were to be updated with a value. Perhaps an attribute like nullType="string" or nullType="number" that when present on a controlled component React will know what to expect when bound to a null (unknown) of a specific type.
Please comment if you think my proposal will fix this issue.
@gaearon suggested creating an RFC for this. Anyone familiar with how to create an RFC if my proposal is sound? I'm somewhat new to React and would like someone more seasoned in doing this to assist. Thanks.

@ericvaladas
Copy link

ericvaladas commented Feb 21, 2019

Has this issue been implemented? I recently discovered that in React 16.3+ a value of undefined or null is treated as "". I was searching to find out if this was a feature or bug and came across this issue.

Examples demonstrating the change:
React 16.2 - https://codesandbox.io/s/j3947rj685
React 16.8 - https://codesandbox.io/s/kkwpjyq653

I have a concern with how this impacts checkbox type inputs. When a checkbox does not have a value attribute set, the value should be set to "on" when checked. However, if I were to render a checkbox with a dynamic value, <input type="checkbox" value={this.props.value}/> for example, and that value is undefined, it will set the value to "". I feel like this will surprise people if they are expecting the value to be "on" and see that the value is "" when checked.

@RyanNerd
Copy link

@ericvaladas Your sandbox examples appear to be using refs and not controlled components.

Last comment from @gaearon on this issue was:

My takeaway so far is that changing the semantics of null as suggested in some comments in this thread will not reduce the confusion.

So it surprises me if React has updated how nulls or undefined are handled.

@JohnGalt1717
Copy link

There is a difference between undefined and null.

React MUST allow null entry (which literally means in every database system that it will be hooked up to "no entry") on form fields. This is well understood in JSON as well.

undefined should be used to mean uncontrolled. Null is controlled and should not be confused with empty strings either. The 2 are very different. An empty string means an entry with no characters. Null means NO ENTRY which are VERY different from eachother both in signaling intent AND in storage.

This is VERY clear in Typescript where I would typically define an optional text field as:

OptionalField: string | null

This would require that it isn't undefined.

This would be undefined or string:

OptionalField?: string

While that is nicer for typescript it may not be desirable in React, but pick either undefined or null as uncontrolled and stick with it so that we know one way or another. I would guess at this point to not break anything that undefined has to equal uncontrolled and explicit NULL set would still be controlled and mean NO ENTRY.

Microsoft for DECADES had this very same problem where Date Pickers in VB 6 and then .NET WinForms wouldn't accept null so you were FORCED to have an entry value because somehow someone in Microsoft couldn't figure out that "No Entry" is entirely valid on a field. It took them until WPF to get their act together and do it right.

@sophiebits
Copy link
Collaborator

See @gaearon's last comment: #11417 (comment). At this time it's unlikely we'll see any changes to this because the new behavior is not clearly better.

I'm going to lock this issue; please file a new one if you are having a problem you can't solve using the information already in this thread or file an RFC with a detailed motivation and explanation if you have a concrete proposal about how to change the behavior.

@facebook facebook locked and limited conversation to collaborators Mar 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

13 participants