facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.09k stars 46.87k forks source link

Treat value={null} as empty string #11417

Open IndifferentDisdain opened 7 years ago

IndifferentDisdain commented 7 years ago

Per @gaearon's request, I'm opening up a new issue based on https://github.com/facebook/react/issues/5013#issuecomment-340898727.

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 commented 7 years ago

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

sophiebits commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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:

// however null instanceof Object; // false

* [relational operators for `null` are not sane](https://github.com/denysdovhan/wtfjs#null-and-relational-operators) 

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

See @gaearon's last comment: https://github.com/facebook/react/issues/11417#issuecomment-413240002. 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.