Avoid setting empty value on reset & submit inputs (#12780)
* Avoid setting empty value on reset & submit inputs * Update ReactDOMFiberInput.js * More test coverage
Ellis Clayton committed
Aug 17, 2018 at 00:19 UTC
9832a1b6d5df06c3d2a54febe2ab3de43ace02fc
2 files changed
+148
-5
packages/react-dom/src/__tests__/ReactDOMInput-test.js
+131
-4
@@ -752,15 +752,142 @@ describe('ReactDOMInput', () => {
752
753
it('should not set a value for submit buttons unnecessarily', () => {
754
const stub = <input type="submit" />;
755
- const node = ReactDOM.render(stub, container);
755
+ ReactDOM.render(stub, container);
756
+ const node = container.firstChild;
757
758
// The value shouldn't be '', or else the button will have no text; it
759
// should have the default "Submit" or "Submit Query" label. Most browsers
760
// report this as not having a `value` attribute at all; IE reports it as
761
// the actual label that the user sees.
761
- expect(
762
- !node.hasAttribute('value') || node.getAttribute('value').length > 0,
763
- ).toBe(true);
762
+ expect(node.hasAttribute('value')).toBe(false);
763
+ });
764
+
765
+ it('should remove the value attribute on submit inputs when value is updated to undefined', () => {
766
+ const stub = <input type="submit" value="foo" onChange={emptyFunction} />;
767
+ ReactDOM.render(stub, container);
768
+
769
+ // Not really relevant to this particular test, but changing to undefined
770
+ // should nonetheless trigger a warning
771
+ expect(() =>
772
+ ReactDOM.render(
773
+ <input type="submit" value={undefined} onChange={emptyFunction} />,
774
+ container,
775
+ ),
776
+ ).toWarnDev(
777
+ 'A component is changing a controlled input of type ' +
778
+ 'submit to be uncontrolled.',
779
+ );
780
+
781
+ const node = container.firstChild;
782
+ expect(node.getAttribute('value')).toBe(null);
783
+ });
784
+
785
+ it('should remove the value attribute on reset inputs when value is updated to undefined', () => {
786
+ const stub = <input type="reset" value="foo" onChange={emptyFunction} />;
787
+ ReactDOM.render(stub, container);
788
+
789
+ // Not really relevant to this particular test, but changing to undefined
790
+ // should nonetheless trigger a warning
791
+ expect(() =>
792
+ ReactDOM.render(
793
+ <input type="reset" value={undefined} onChange={emptyFunction} />,
794
+ container,
795
+ ),
796
+ ).toWarnDev(
797
+ 'A component is changing a controlled input of type ' +
798
+ 'reset to be uncontrolled.',
799
+ );
800
+
801
+ const node = container.firstChild;
802
+ expect(node.getAttribute('value')).toBe(null);
803
+ });
804
+
805
+ it('should set a value on a submit input', () => {
806
+ let stub = <input type="submit" value="banana" />;
807
+ ReactDOM.render(stub, container);
808
+ const node = container.firstChild;
809
+
810
+ expect(node.getAttribute('value')).toBe('banana');
811
+ });
812
+
813
+ it('should not set an undefined value on a submit input', () => {
814
+ let stub = <input type="submit" value={undefined} />;
815
+ ReactDOM.render(stub, container);
816
+ const node = container.firstChild;
817
+
818
+ // Note: it shouldn't be an empty string
819
+ // because that would erase the "submit" label.
820
+ expect(node.getAttribute('value')).toBe(null);
821
+
822
+ ReactDOM.render(stub, container);
823
+ expect(node.getAttribute('value')).toBe(null);
824
+ });
825
+
826
+ it('should not set an undefined value on a reset input', () => {
827
+ let stub = <input type="reset" value={undefined} />;
828
+ ReactDOM.render(stub, container);
829
+ const node = container.firstChild;
830
+
831
+ // Note: it shouldn't be an empty string
832
+ // because that would erase the "reset" label.
833
+ expect(node.getAttribute('value')).toBe(null);
834
+
835
+ ReactDOM.render(stub, container);
836
+ expect(node.getAttribute('value')).toBe(null);
837
+ });
838
+
839
+ it('should not set a null value on a submit input', () => {
840
+ let stub = <input type="submit" value={null} />;
841
+ expect(() => {
842
+ ReactDOM.render(stub, container);
843
+ }).toWarnDev('`value` prop on `input` should not be null');
844
+ const node = container.firstChild;
845
+
846
+ // Note: it shouldn't be an empty string
847
+ // because that would erase the "submit" label.
848
+ expect(node.getAttribute('value')).toBe(null);
849
+
850
+ ReactDOM.render(stub, container);
851
+ expect(node.getAttribute('value')).toBe(null);
852
+ });
853
+
854
+ it('should not set a null value on a reset input', () => {
855
+ let stub = <input type="reset" value={null} />;
856
+ expect(() => {
857
+ ReactDOM.render(stub, container);
858
+ }).toWarnDev('`value` prop on `input` should not be null');
859
+ const node = container.firstChild;
860
+
861
+ // Note: it shouldn't be an empty string
862
+ // because that would erase the "reset" label.
863
+ expect(node.getAttribute('value')).toBe(null);
864
+
865
+ ReactDOM.render(stub, container);
866
+ expect(node.getAttribute('value')).toBe(null);
867
+ });
868
+
869
+ it('should set a value on a reset input', () => {
870
+ let stub = <input type="reset" value="banana" />;
871
+ ReactDOM.render(stub, container);
872
+ const node = container.firstChild;
873
+
874
+ expect(node.getAttribute('value')).toBe('banana');
875
+ });
876
+
877
+ it('should set an empty string value on a submit input', () => {
878
+ let stub = <input type="submit" value="" />;
879
+ ReactDOM.render(stub, container);
880
+ const node = container.firstChild;
881
+
882
+ expect(node.getAttribute('value')).toBe('');
883
+ });
884
+
885
+ it('should set an empty string value on a reset input', () => {
886
+ let stub = <input type="reset" value="" />;
887
+ ReactDOM.render(stub, container);
888
+ const node = container.firstChild;
889
+
890
+ expect(node.getAttribute('value')).toBe('');
891
});
892
893
it('should control radio buttons', () => {
packages/react-dom/src/client/ReactDOMFiberInput.js
+17
-1
@@ -172,9 +172,10 @@ export function updateWrapper(element: Element, props: Object) {
172
updateChecked(element, props);
173
174
const value = getToStringValue(props.value);
175
+ const type = props.type;
176
177
if (value != null) {
177
- if (props.type === 'number') {
178
+ if (type === 'number') {
179
if (
180
(value === 0 && node.value === '') ||
181
// We explicitly want to coerce to number here if possible.
@@ -186,6 +187,11 @@ export function updateWrapper(element: Element, props: Object) {
187
} else if (node.value !== toString(value)) {
188
node.value = toString(value);
189
}
190
+ } else if (type === 'submit' || type === 'reset') {
191
+ // Submit/reset inputs need the attribute removed completely to avoid
192
+ // blank-text buttons.
193
+ node.removeAttribute('value');
194
+ return;
195
}
196
197
if (props.hasOwnProperty('value')) {
@@ -207,6 +213,16 @@ export function postMountWrapper(
213
const node = ((element: any): InputWithWrapperState);
214
215
if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
216
+ // Avoid setting value attribute on submit/reset inputs as it overrides the
217
+ // default value provided by the browser. See: #12872
218
+ const type = props.type;
219
+ if (
220
+ (type === 'submit' || type === 'reset') &&
221
+ (props.value === undefined || props.value === null)
222
+ ) {
223
+ return;
224
+ }
225
+
226
const initialValue = toString(node._wrapperState.initialValue);
227
const currentValue = node.value;
228