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

ReactLink does not update input when state changes to null or undefined #1678

Closed
brycekahle opened this issue Jun 11, 2014 · 6 comments
Closed

Comments

@brycekahle
Copy link

http://jsfiddle.net/q8K9A/1/

If you reset a state variable to null or undefined the input control doesn't change. You have to reset to empty string '' for it to change.

@chenglou
Copy link
Contributor

Another quirk: if you type X, set it to empty string, then to undefined, X comes back in the field. Same for without ReactLink.

Generally, what should be the correct behavior of <input value={null} /> or <input value={undefined} />? Should it become an uncontrolled component (current behavior), or a controlled component with value=''?

@sebmarkbage @spicyj

@syranide
Copy link
Contributor

@chenglou Intuitively it seems like it should become uncontrolled and set to defaultValue, if value is set to null/undefined. However, if you're using ReactLink with a value of undefined/null it's kind of a gray area. Although I still think it may make sense to mirror the above behavior of value exactly.

That makes the most sense to me at least, it also seems favorable from a deterministic perspective. If the input was keyed and given a new key at the same time as becoming uncontrolled, that's the behavior it would have. So it makes sense to mirror it, IMHO.

Not setting it to defaultValue but just keeping the last value also makes sense, but I can't imagine a use-case where it would be a good idea to rely on that implicit behavior.

@tgriesser
Copy link

Yeah this one just confused me for about 20 minutes or so, seems like the behavior @syranide described would make sense here. Or at least a warning if the current behavior is desired.

@jhford
Copy link

jhford commented Sep 29, 2014

I just wasted a bunch of time working around this myself trying to clear a text input box. I wonder if the documentation could be updated to highlight this behaviour, maybe by adding a clear button to the Todo app in the getting start part of the docs.

@syranide
Copy link
Contributor

Reading this again I'm pretty sure the technically "correct" behavior is for it to keep the value (like it does). If value (or valueLink by association) is not set then input is uncontrolled and it simply should not mutate the value, regardless of what it was.

1. <input type="text" value="forced" defaultValue="initial1" />
2. <input type="text" defaultValue="initial2" />
3. <input type="text" />

If you consider those as a sequence 1 -> 2 -> 3, it makes sense (technically) to me that the value would be forced from start to finish. When a component goes from controlled to uncontrolled, it stops being controlled, it does not reset/remount. Intuitively it could make sense that it should become initial2 for 2 and 3, but it would mean that an input would figuratively reset/remount when transitioning from controlled to uncontrolled.

PS. But I think mixed controlled and uncontrolled is probably a flaw by its own, you should use one or the other. Or at the least never transition between them (kind of related #2255).

@jimfb
Copy link
Contributor

jimfb commented Sep 30, 2015

As per #2302 the current plan is to pull linking out of controlled components, but as discussed in #2302 it's easy to create wrappers that expose any behavior (including the undefined/null reset behavior described in this issue). If you find this behavior useful, I would encourage you to publish a npm module or git repository that provides this functionality as a wrapper component so other people can use it. Since this is not something we plan on supporting in the core, I'm going to go ahead and close out the issue.

@jimfb jimfb closed this as completed Sep 30, 2015
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

7 participants