Firefox validation triggers on input component render

[outdooricon]

When using native form validation and loading a page with a form containing an input with the required prop set, it triggers browser validation immediately. This produces Firefox's native error highlighting. This is because Firefox appears to run validation when the DOM is used to set a value on an existing input component.

The behavior can be seen here: http://codepen.io/anon/pen/zowOzo

Angular has run into this already.

[peternewnham]

This appears to have been introduced in 15.2 as a result of #6406. This line -https://github.com/facebook/react/blob/master/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js#L236 - is setting the value of the node explicitly to detach it from the defaultValue but doing so triggers validation which firefox displays feedback on immediately.

Textareas also suffer from the same problem.

I am happy to spend more time working on a solution if this is considered to be an actual React issue and not a problem with Firefox itself.

[QuantumHive]

Can confirm this. See animated gif:

[gaearon]

@aweary Any thoughts on this?

[jquense]

The issue seems to be this line: https://github.com/facebook/react/blob/master/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js#L265 That seems like it should be a noop in all cases and lack of comment makes it a bit tough to understand why it's even there...

From what I can see other sets on value are guarded so I think this is the only space but i'm a bit afraid to change it without understanding why it's there

[nhunzaker]

@jquense I know something about this. There is a side-effect to input.value = input.value.

This produces something called "value detachment" (not my term, I stole it from @sophiebits). Assigning the value attribute or input.defaultProp changes the value property. Check out this codepen:

https://codepen.io/nhunzaker/pen/zEpbWK?editors=1010

Still this seems fixable. We should map out all of the possible sequences of assignments and see if we can easily avoid needing to do this.

It's also worth identifying if the line above it forces validation of required date inputs too:
https://github.com/facebook/react/blob/master/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js#L259-L263

[sophiebits]

Don't think that was my term either. :) Maybe jimfb.

[nhunzaker]

Looks like we still need this for the following test:

it('should update `defaultValue` for uncontrolled input', () => {
  var container = document.createElement('div');

  var node = ReactDOM.render(
    <input type="text" defaultValue="0" />,
    container,
  );

  expect(node.value).toBe('0');

  ReactDOM.render(<input type="text" defaultValue="1" />, container);

  expect(node.value).toBe('0');
  expect(node.defaultValue).toBe('1');
});

If I remove the line (node.value = node.value), setting the defaultValue from "0" to "1" changes the value property from "0" to "1". This is an error, as far as React is concerned.

I think it could be debated if this is the correct behavior, but it's what we have now. If we were to remove this check, defaultValue would change if it was set to a different value only until the user interacted with it. If we can determine defaultValue is relatively static, this may be a reasonable trade off.

Alternatively, I wonder if we could only perform this "detachment" if the input has never changed and the input receives a new defaultValue prop, but this sounds very mechanical and complicated.

[jquense]

i just love how html has no api for changing the validation state...

[nhunzaker]

I'm on this. I almost have a fix here:
master...nhunzaker:firefox-validation

I'm just investigating an unhelpful error in IE10 when I set node.type. I'll pick that up tomorrow.

[nhunzaker]

Huh. Got it. You have to use setAttribute('type', 'x') instead of node.type = 'x'.

This is out for review in: #11202

[nhunzaker]

This was fixed with #11746

[somebody32]

@nhunzaker I believe that is fixing only uncontrolled inputs, here is an example of controlled one and validation is still firing: https://codepen.io/anon/pen/ZREQvv

[nhunzaker]

Ah, you are correct. Thank you. I'll send out a PR for this shortly.

[nhunzaker]

Sent out a new PR here:
#12925

Now I'll start checking off the list of browsers to test in the PR description.

[nhunzaker]

With #12925 (review) merged, this should not be an issue in the next release.

Thanks @somebody32 for the test case! It was super helpful!

[somebody32]

Thank you @nhunzaker for an enormously fast solution!

[gaearon]

Fixed in React 16.4.1.