Warning when changing the type and value of an input field

[mathisonian]

In my render method I have something like

<input type={dynamicTypeValue} value={dynamicValue} />

If I first render this input as a number, (e.g. dynamicTypeValue = 'number'; dynamicValue = 5) but then change the input to a string: (dynamicTypeValue = 'string'; dynamicValue = '01/01/2016') I get a warning
that the new value is not a valid number:

The specified value "01/01/2000" is not a valid number. The value must match to the following regular expression: -?(\d+|\d+\.\d+|\.\d+)([eE][-+]?\d+)?
DOMPropertyOperations.js:142 The specified value "01/01/2012" is not a valid number. The value must match to the following regular expression: -?(\d+|\d+\.\d+|\.\d+)([eE][-+]?\d+)?

Is this expected behavior?

[jimfb]

Looks like we're applying the prop changes in a bad order. The logic to get this right is solvable in general (for known types), but could be complex to get perfect if we want to minimize DOM operations.

[sophiebits]

I'm surprised that this particular case is problematic because this code should ensure that we always set .type before any other attributes on an <input>:

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

Line 74 in 0b1fd18

type: undefined,

Not sure why that isn't working as designed. It's not totally clear to me whether setting type first always solves cases like these or setting type first would simply cause the same warning when switching the other way.

(See also #2242, which is something we may want to have happen but may be difficult to implement cleanly in the current system. It was easier when we had full composite wrappers for these components but we no longer do. Creating a new element whenever the type changes could be surprising because it would mean that the ref to that component changes during the lifetime of the component which we never have elsewhere.)

[sophiebits]

The easiest workaround here is to set a key on the input that changes with the type so that a new input element is created when the type changes.

[gaearon]

Could be caused by Object.assign V8 order bug? Or was this before 15?

[gurinderhans]

@gaearon this is still in the 15 release

[jimfb]

@gurinderhans Which browser? Also, can you provide a jsfiddle that demonstrates the issue?

[gurinderhans]

@jimfb Here you go.
Browser: Chrome 50.0.2661.86 (64-bit)
JSFiddle: https://jsfiddle.net/mb90na04/1/

[gurinderhans]

Following the chrome debugger I found this:
There's this line of code, https://github.com/facebook/react/blob/master/src/renderers/dom/shared/ReactDOMComponent.js#L829, which after makes a call to https://github.com/facebook/react/blob/master/src/renderers/dom/client/wrappers/ReactDOMInput.js#L221 and then the value of the input is tried to be changed while _updateDOMProperties hasn't yet been called to update the element type attribute, thus the warning is generated. Once _updateDOMProperties is called, type gets set before value, like it is suppose to and everything goes according to plan.

PS: Of Course, removing the call to ReactDOMInput.updateWrapper, inside the switch case rids the warning, but this may be required for some other cases as I also notice it gets called if element type is textarea.

You could check for type change, and not set value, or update type and then set value inside ReactDOMInput.updateWrapper. I'm also wondering why the call is required for cases like <input> or <textarea>

[aweary]

Here is a simpler case reproducing the issue: https://jsfiddle.net/97gr5e65/1/

It seems to only happen if changing from number to text. I'm also not able to reproduce the warning in Safari or Firefox on OS X. It also doesn't seem to occur using ReactTestUtils.

[nhunzaker]

This was an interesting bug to check out, so I did some poking around. My initial thought was just to assign type before value in the updateWrapper. Is this sane?

https://github.com/facebook/react/compare/master...nhunzaker:nh-input-change-fix?expand=1

It eliminates the bug, but it's too simple. It feels shallow. What do you think?

[sophiebits]

Can we move the .updateWrapper calls below the _updateDOMChildren call (splitting them out from the getNativeProps calls)? I think that's the best solution here and looks like it should work.

[nhunzaker]

Yes. I was also able to get test coverage on this. The value is nullified by the DOM when the type changes such that the the value is no longer valid, or if a new value is assigned that is not valid.

Unrelated, it's pretty cool that JSDOM picks this up.

Done in #7333

[mariorodriguespt]

Still happens with version 16.4.1