Avoid setting empty value on reset & submit inputs (#12780)

* Avoid setting empty value on reset & submit inputs

* Update ReactDOMFiberInput.js

* More test coverage

Author: ellsclytn

packages/react-dom/src/__tests__/ReactDOMInput-test.js

@@ -752,15 +752,142 @@ describe('ReactDOMInput', () => {
 
   it('should not set a value for submit buttons unnecessarily', () => {
     const stub = <input type="submit" />;
-    const node = ReactDOM.render(stub, container);
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
 
     // The value shouldn't be '', or else the button will have no text; it
     // should have the default "Submit" or "Submit Query" label. Most browsers
     // report this as not having a `value` attribute at all; IE reports it as
     // the actual label that the user sees.
-    expect(
-      !node.hasAttribute('value') || node.getAttribute('value').length > 0,
-    ).toBe(true);
+    expect(node.hasAttribute('value')).toBe(false);
+  });
+
+  it('should remove the value attribute on submit inputs when value is updated to undefined', () => {
+    const stub = <input type="submit" value="foo" onChange={emptyFunction} />;
+    ReactDOM.render(stub, container);
+
+    // Not really relevant to this particular test, but changing to undefined
+    // should nonetheless trigger a warning
+    expect(() =>
+      ReactDOM.render(
+        <input type="submit" value={undefined} onChange={emptyFunction} />,
+        container,
+      ),
+    ).toWarnDev(
+      'A component is changing a controlled input of type ' +
+        'submit to be uncontrolled.',
+    );
+
+    const node = container.firstChild;
+    expect(node.getAttribute('value')).toBe(null);
+  });
+
+  it('should remove the value attribute on reset inputs when value is updated to undefined', () => {
+    const stub = <input type="reset" value="foo" onChange={emptyFunction} />;
+    ReactDOM.render(stub, container);
+
+    // Not really relevant to this particular test, but changing to undefined
+    // should nonetheless trigger a warning
+    expect(() =>
+      ReactDOM.render(
+        <input type="reset" value={undefined} onChange={emptyFunction} />,
+        container,
+      ),
+    ).toWarnDev(
+      'A component is changing a controlled input of type ' +
+        'reset to be uncontrolled.',
+    );
+
+    const node = container.firstChild;
+    expect(node.getAttribute('value')).toBe(null);
+  });
+
+  it('should set a value on a submit input', () => {
+    let stub = <input type="submit" value="banana" />;
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
+
+    expect(node.getAttribute('value')).toBe('banana');
+  });
+
+  it('should not set an undefined value on a submit input', () => {
+    let stub = <input type="submit" value={undefined} />;
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
+
+    // Note: it shouldn't be an empty string
+    // because that would erase the "submit" label.
+    expect(node.getAttribute('value')).toBe(null);
+
+    ReactDOM.render(stub, container);
+    expect(node.getAttribute('value')).toBe(null);
+  });
+
+  it('should not set an undefined value on a reset input', () => {
+    let stub = <input type="reset" value={undefined} />;
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
+
+    // Note: it shouldn't be an empty string
+    // because that would erase the "reset" label.
+    expect(node.getAttribute('value')).toBe(null);
+
+    ReactDOM.render(stub, container);
+    expect(node.getAttribute('value')).toBe(null);
+  });
+
+  it('should not set a null value on a submit input', () => {
+    let stub = <input type="submit" value={null} />;
+    expect(() => {
+      ReactDOM.render(stub, container);
+    }).toWarnDev('`value` prop on `input` should not be null');
+    const node = container.firstChild;
+
+    // Note: it shouldn't be an empty string
+    // because that would erase the "submit" label.
+    expect(node.getAttribute('value')).toBe(null);
+
+    ReactDOM.render(stub, container);
+    expect(node.getAttribute('value')).toBe(null);
+  });
+
+  it('should not set a null value on a reset input', () => {
+    let stub = <input type="reset" value={null} />;
+    expect(() => {
+      ReactDOM.render(stub, container);
+    }).toWarnDev('`value` prop on `input` should not be null');
+    const node = container.firstChild;
+
+    // Note: it shouldn't be an empty string
+    // because that would erase the "reset" label.
+    expect(node.getAttribute('value')).toBe(null);
+
+    ReactDOM.render(stub, container);
+    expect(node.getAttribute('value')).toBe(null);
+  });
+
+  it('should set a value on a reset input', () => {
+    let stub = <input type="reset" value="banana" />;
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
+
+    expect(node.getAttribute('value')).toBe('banana');
+  });
+
+  it('should set an empty string value on a submit input', () => {
+    let stub = <input type="submit" value="" />;
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
+
+    expect(node.getAttribute('value')).toBe('');
+  });
+
+  it('should set an empty string value on a reset input', () => {
+    let stub = <input type="reset" value="" />;
+    ReactDOM.render(stub, container);
+    const node = container.firstChild;
+
+    expect(node.getAttribute('value')).toBe('');
   });
 
   it('should control radio buttons', () => {

packages/react-dom/src/client/ReactDOMFiberInput.js

@@ -172,9 +172,10 @@ export function updateWrapper(element: Element, props: Object) {
   updateChecked(element, props);
 
   const value = getToStringValue(props.value);
+  const type = props.type;
 
   if (value != null) {
-    if (props.type === 'number') {
+    if (type === 'number') {
       if (
         (value === 0 && node.value === '') ||
         // We explicitly want to coerce to number here if possible.
@@ -186,6 +187,11 @@ export function updateWrapper(element: Element, props: Object) {
     } else if (node.value !== toString(value)) {
       node.value = toString(value);
     }
+  } else if (type === 'submit' || type === 'reset') {
+    // Submit/reset inputs need the attribute removed completely to avoid
+    // blank-text buttons.
+    node.removeAttribute('value');
+    return;
   }
 
   if (props.hasOwnProperty('value')) {
@@ -207,6 +213,16 @@ export function postMountWrapper(
   const node = ((element: any): InputWithWrapperState);
 
   if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
+    // Avoid setting value attribute on submit/reset inputs as it overrides the
+    // default value provided by the browser. See: #12872
+    const type = props.type;
+    if (
+      (type === 'submit' || type === 'reset') &&
+      (props.value === undefined || props.value === null)
+    ) {
+      return;
+    }
+
     const initialValue = toString(node._wrapperState.initialValue);
     const currentValue = node.value;