Properly set value and defaultValue for input and textarea #6406

jimfb · 2016-04-04T17:22:45Z

Spiritual successor to #5680. This is a fix for #6119 and #6219, both of which are blocking v15. As an added bonus, it happens to also fix #4618 :P. It works by setting/removing both the attribute and property for value on inputs, and setting the property for defaultValue.

I fixed the tests to be more reasonable (inputs retain their initial value, rather than having the value potentially change when defaultValue changes). I also fixed the browser inconsistencies.

facebook-github-bot · 2016-04-04T18:27:47Z

@jimfb updated the pull request.

facebook-github-bot · 2016-04-04T18:58:19Z

@jimfb updated the pull request.

facebook-github-bot · 2016-04-04T22:26:23Z

@jimfb updated the pull request.

facebook-github-bot · 2016-04-04T23:39:07Z

@jimfb updated the pull request.

jimfb · 2016-04-04T23:49:37Z

Ok, I think this is ready to go.

EDIT: Actually, never mind, the bug still exists, fixing :/ .

sophiebits · 2016-04-05T00:03:24Z

This is not for 15.0, right?

jimfb · 2016-04-05T00:07:33Z

Paul Sebastian and I were at the PR review, and we weren't 100% decided, but Paul said to fix up #5680 to potentially land for v15.0

facebook-github-bot · 2016-04-05T02:38:48Z

@jimfb updated the pull request.

facebook-github-bot · 2016-04-05T11:05:34Z

@jimfb updated the pull request.

jimfb · 2016-04-05T11:15:23Z

Ok, I think this is good to go now. Here are some fiddles that demonstrate everything working:

http://jsfiddle.net/3r0krefu/ (fixes Ben's concerns from #5680)
http://jsfiddle.net/ub6ec5rt/ (fixes Ben's concerns from #5680)
http://jsfiddle.net/v6fz2ct1/ (fixes #6119)
http://jsfiddle.net/wavkdt6b/ (fixes #6219)
http://jsfiddle.net/c0u6v6fr/ (fixes #4618)

gaearon · 2016-04-05T12:06:37Z

src/renderers/dom/client/wrappers/ReactDOMInput.js

+      var node = ReactDOMComponentTree.getNodeFromInstance(inst);
+      if (node.defaultValue !== props.defaultValue) {
+        var tmp = node.value;
+        node.defaultValue = ''+props.defaultValue;


Nit: missing spaces before and after +

zpao · 2016-04-05T18:15:27Z

Can you refresh me and explain why we need to change defaultValue behavior here?

zpao · 2016-05-25T23:03:08Z

Are we targeting 15 or 16 with this?

jimfb · 2016-06-01T18:39:07Z

I don't have a super strong preference. Should be safe for 15, but whatever works.

zpao · 2016-06-06T20:56:16Z

src/renderers/dom/shared/ReactDOMComponent.js

+          textareaPostMount,
+          this
+        );
+        break;


So I just came back here due to a merge conflict and perhaps I'm missing something but doesn't this change mean we'll stop autofocusing inputs and textareas?

That's what it looks like to me. @jimfb?

…6406) * Have `defaultValue` reach DOM node for html input box for facebook#4618 * Cleanup and bug fixes for merge. (cherry picked from commit 4338c8d)

* Have `defaultValue` reach DOM node for html input box for #4618 * Cleanup and bug fixes for merge. (cherry picked from commit 4338c8d)

TheLudd · 2017-06-08T14:27:02Z

Hello
I noticed that in react 15.2.0 (but not in 15.1.0) there is a change in how checkboxes are rendered. And I suspect it might have something to do with this change.
In version 15.2.0 rendered checkbox inputs will always have the property value="on" unless value is specified to be something else.
I wonder if this is intentional and/or wanted. It causes errors in my application/lib where I make assumptions based on if a checkbox has a value set or not. Now they always have values...

gaearon · 2017-06-08T14:37:48Z

If you have an issue with the latest version please file a new issue. Thanks!
Maybe @aweary or @nhunzaker could help with older versions.

zyy7259 · 2018-03-15T07:02:57Z

src/renderers/dom/client/wrappers/ReactDOMInput.js

+    // This is in postMount because we need access to the DOM node, which is not
+    // available until after the component has mounted.
+    var node = ReactDOMComponentTree.getNodeFromInstance(inst);
+    node.value = node.value; // Detach value from defaultValue


I'm trying to understand the logic here. Could somebody tell what "detach" means here?

If I recall from way back when, once you call the setter for value, it then ignores changes to the defaultValue. In this case, we set the value to the same value, which has this effect.

Thanks for the explanation

jimfb force-pushed the attribute-for-value branch from d7ca7f6 to 574328a Compare April 4, 2016 17:29

facebook-github-bot added GH Review: review-needed CLA Signed labels Apr 4, 2016

jimfb force-pushed the attribute-for-value branch from e788142 to 4ba86d1 Compare April 4, 2016 17:41

jimfb force-pushed the attribute-for-value branch from 3714855 to 4ba86d1 Compare April 4, 2016 18:47

jimfb force-pushed the attribute-for-value branch 2 times, most recently from bec734c to 9aabd4c Compare April 4, 2016 22:21

jimfb force-pushed the attribute-for-value branch 4 times, most recently from d049ebf to 1e8a3e8 Compare April 4, 2016 23:38

jimfb force-pushed the attribute-for-value branch 2 times, most recently from ff1e9d8 to 3186f4c Compare April 5, 2016 02:30

jimfb force-pushed the attribute-for-value branch from 3186f4c to fef7028 Compare April 5, 2016 11:01

jimfb force-pushed the attribute-for-value branch from fef7028 to 8c06195 Compare April 5, 2016 11:13

jimfb force-pushed the attribute-for-value branch from 8c06195 to 9eeca54 Compare April 5, 2016 11:42

gaearon reviewed Apr 5, 2016
View reviewed changes

jimfb force-pushed the attribute-for-value branch 2 times, most recently from 355bf75 to f6c393c Compare April 5, 2016 12:39

zpao added the semver-minor label Jun 1, 2016

zpao added this to the 15-next milestone Jun 1, 2016

zpao mentioned this pull request Jun 6, 2016

Remove HAS_SIDE_EFFECTS logic #6979

Closed

zpao reviewed Jun 6, 2016
View reviewed changes

This was referenced Jun 8, 2016

<progress> requires value to be an attribute, but it's a property due to inputs #6119

Closed

Set value by setAttribute(value) for html input box for #4618 #5680

Closed

zpao pushed a commit that referenced this pull request Jun 14, 2016

Properly set value and defaultValue for input and textarea (#6406)

f98e6c1

* Have `defaultValue` reach DOM node for html input box for #4618 * Cleanup and bug fixes for merge. (cherry picked from commit 4338c8d)

zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016

yoavf mentioned this pull request Jun 19, 2016

i18n: community translator is broken Automattic/wp-calypso#6139

Closed

kostasmanionis mentioned this pull request Jul 3, 2016

Form submit button has empty values in 15.2.0 #7179

Closed

syranide mentioned this pull request Jul 6, 2016

Input elements always rendered with name attribute #7198

Closed

zpao mentioned this pull request Jul 12, 2016

Bug: Backspace in input type="number" behaves badly in Blink #7253

Closed

sophiebits mentioned this pull request Jul 19, 2016

Date input with defaultValue regression in 15.2 #7233

Closed

aweary mentioned this pull request Aug 13, 2016

Controlled input with type=email produces console warnings with Chrome 52 #7487

Closed

jdalton mentioned this pull request Nov 10, 2016

IE 11 and Edge no longer prompt to remember password on controlled form #7328

Open

peternewnham mentioned this pull request Dec 4, 2016

Firefox validation triggers on input component render #8395

Closed

MoGray mentioned this pull request Mar 11, 2017

IE11: Placeholder of the textarea will only show up after focus and blur Andarist/react-textarea-autosize#125

Closed

syranide mentioned this pull request Jul 12, 2017

Controlled inputs do not synchronize value or checked attribute #10150

Closed

renovate bot mentioned this pull request Feb 2, 2018

Update dependency react to v0.14.9 - autoclosed signavio/backbone-rel#29

Closed

1 task

zyy7259 reviewed Mar 15, 2018

View reviewed changes

This was referenced Aug 31, 2018

React Fire: Modernizing React DOM #13525

Closed

🔥 Stop syncing the value attribute on inputs (behind a feature flag) #13526

Merged

Properly set value and defaultValue for input and textarea #6406

Properly set value and defaultValue for input and textarea #6406

Uh oh!

Conversation

jimfb commented Apr 4, 2016

Uh oh!

facebook-github-bot commented Apr 4, 2016

Uh oh!

facebook-github-bot commented Apr 4, 2016

Uh oh!

facebook-github-bot commented Apr 4, 2016

Uh oh!

facebook-github-bot commented Apr 4, 2016

Uh oh!

jimfb commented Apr 4, 2016

Uh oh!

sophiebits commented Apr 5, 2016

Uh oh!

jimfb commented Apr 5, 2016

Uh oh!

facebook-github-bot commented Apr 5, 2016

Uh oh!

facebook-github-bot commented Apr 5, 2016

Uh oh!

jimfb commented Apr 5, 2016

Uh oh!

gaearon Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

zpao commented Apr 5, 2016

Uh oh!

zpao commented May 25, 2016

Uh oh!

jimfb commented Jun 1, 2016

Uh oh!

zpao Jun 6, 2016

Choose a reason for hiding this comment

Uh oh!

sophiebits Jun 7, 2016

Choose a reason for hiding this comment

Uh oh!

jimfb Jun 7, 2016

Choose a reason for hiding this comment

Uh oh!

TheLudd commented Jun 8, 2017

Uh oh!

gaearon commented Jun 8, 2017

Uh oh!

zyy7259 Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

hayesgm Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

zyy7259 Mar 16, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants