Merge pull request #5048 from antoaravinth/PR-5013

Issue: 5013 Added necessary code for firing warning if value is null
facebook · Oct 15, 2015 · b735dd4 · b735dd4
2 parents 8168c8e + b42c1da
commit b735dd4
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 1 deletion.
diff --git a/src/renderers/dom/client/wrappers/ReactDOMInput.js b/src/renderers/dom/client/wrappers/ReactDOMInput.js
@@ -24,6 +24,7 @@ var instancesByReactID = {};
 
 var didWarnValueLink = false;
 var didWarnCheckedLink = false;
+var didWarnValueNull = false;
 
 function forceUpdateIfMounted() {
   if (this._rootNodeID) {
@@ -32,6 +33,19 @@ function forceUpdateIfMounted() {
   }
 }
 
+function warnIfValueIsNull(props) {
+  if (props != null && props.value === null && !didWarnValueNull) {
+    warning(
+        false,
+        '`value` prop on `input` should not be null. ' +
+        'Consider using the empty string to clear the component or' +
+        ' `undefined` for uncontrolled components.'
+    );
+
+    didWarnValueNull = true;
+  }
+}
+
 /**
  * Implements an <input> native component that allows setting these optional
  * props: `checked`, `value`, `defaultChecked`, and `defaultValue`.
@@ -86,6 +100,7 @@ var ReactDOMInput = {
         );
         didWarnCheckedLink = true;
       }
+      warnIfValueIsNull(props);
     }
 
     var defaultValue = props.defaultValue;
@@ -108,6 +123,10 @@ var ReactDOMInput = {
   updateWrapper: function(inst) {
     var props = inst._currentElement.props;
 
+    if (__DEV__) {
+      warnIfValueIsNull(props);
+    }
+
     // TODO: Shouldn't this be getChecked(props)?
     var checked = props.checked;
     if (checked != null) {

diff --git a/src/renderers/dom/client/wrappers/ReactDOMSelect.js b/src/renderers/dom/client/wrappers/ReactDOMSelect.js
@@ -19,6 +19,7 @@ var assign = require('Object.assign');
 var warning = require('warning');
 
 var didWarnValueLink = false;
+var didWarnValueNull = false;
 
 function updateOptionsIfPendingUpdateAndMounted() {
   if (this._rootNodeID && this._wrapperState.pendingUpdate) {
@@ -43,6 +44,19 @@ function getDeclarationErrorAddendum(owner) {
   return '';
 }
 
+function warnIfValueIsNull(props) {
+  if (props != null && props.value === null && !didWarnValueNull) {
+    warning(
+        false,
+        '`value` prop on `select` should not be null. ' +
+        'Consider using the empty string to clear the component or' +
+        ' `undefined` for uncontrolled components.'
+    );
+
+    didWarnValueNull = true;
+  }
+}
+
 var valuePropNames = ['value', 'defaultValue'];
 
 /**
@@ -153,6 +167,7 @@ var ReactDOMSelect = {
   mountWrapper: function(inst, props) {
     if (__DEV__) {
       checkSelectPropTypes(inst, props);
+      warnIfValueIsNull(props);
     }
 
     var value = LinkedValueUtils.getValue(props);
@@ -172,6 +187,9 @@ var ReactDOMSelect = {
 
   postUpdateWrapper: function(inst) {
     var props = inst._currentElement.props;
+    if (__DEV__) {
+      warnIfValueIsNull(props);
+    }
 
     // After the initial mount, we control selected-ness manually so don't pass
     // this value down

diff --git a/src/renderers/dom/client/wrappers/ReactDOMTextarea.js b/src/renderers/dom/client/wrappers/ReactDOMTextarea.js
@@ -20,6 +20,7 @@ var invariant = require('invariant');
 var warning = require('warning');
 
 var didWarnValueLink = false;
+var didWarnValueNull = false;
 
 function forceUpdateIfMounted() {
   if (this._rootNodeID) {
@@ -28,6 +29,19 @@ function forceUpdateIfMounted() {
   }
 }
 
+function warnIfValueIsNull(props) {
+  if (props != null && props.value === null && !didWarnValueNull) {
+    warning(
+        false,
+        '`value` prop on `textarea` should not be null. ' +
+        'Consider using the empty string to clear the component or' +
+        ' `undefined` for uncontrolled components.'
+    );
+
+    didWarnValueNull = true;
+  }
+}
+
 /**
  * Implements a <textarea> native component that allows setting `value`, and
  * `defaultValue`. This differs from the traditional DOM API because value is
@@ -76,6 +90,7 @@ var ReactDOMTextarea = {
         );
         didWarnValueLink = true;
       }
+      warnIfValueIsNull(props);
     }
 
     var defaultValue = props.defaultValue;
@@ -107,7 +122,6 @@ var ReactDOMTextarea = {
       defaultValue = '';
     }
     var value = LinkedValueUtils.getValue(props);
-
     inst._wrapperState = {
       // We save the initial value so that `ReactDOMComponent` doesn't update
       // `textContent` (unnecessary since we update value).
@@ -120,6 +134,11 @@ var ReactDOMTextarea = {
 
   updateWrapper: function(inst) {
     var props = inst._currentElement.props;
+
+    if (__DEV__) {
+      warnIfValueIsNull(props);
+    }
+
     var value = LinkedValueUtils.getValue(props);
     if (value != null) {
       // Cast `value` to a string to ensure the value is set correctly. While

diff --git a/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
@@ -403,4 +403,15 @@ describe('ReactDOMInput', function() {
       <input type="checkbox" checkedLink={link} valueLink={emptyFunction} />;
     expect(() => ReactDOM.render(instance, node)).toThrow();
   });
+
+  it('should throw warning message if value is null', function() {
+    ReactTestUtils.renderIntoDocument(<input type="text" value={null} />);
+    expect(console.error.argsForCall[0][0]).toContain(
+        '`value` prop on `input` should not be null. Consider using the empty string to clear the component or'
+        + ' `undefined` for uncontrolled components.'
+    );
+
+    ReactTestUtils.renderIntoDocument(<input type="text" value={null} />);
+    expect(console.error.argsForCall.length).toBe(1);
+  });
 });
diff --git a/src/renderers/dom/client/wrappers/__tests__/ReactDOMSelect-test.js b/src/renderers/dom/client/wrappers/__tests__/ReactDOMSelect-test.js
@@ -463,4 +463,17 @@ describe('ReactDOMSelect', function() {
     expect(node.options[1].selected).toBe(false);  // giraffe
     expect(node.options[2].selected).toBe(false);  // gorilla
   });
+
+  it('should throw warning message if value is null', function() {
+    spyOn(console, 'error');
+
+    ReactTestUtils.renderIntoDocument(<select value={null}><option value="test"/></select>);
+    expect(console.error.argsForCall[0][0]).toContain(
+        '`value` prop on `select` should not be null. Consider using the empty string to clear the component or'
+        + ' `undefined` for uncontrolled components.'
+    );
+
+    ReactTestUtils.renderIntoDocument(<select value={null}><option value="test"/></select>);
+    expect(console.error.argsForCall.length).toBe(1);
+  });
 });
diff --git a/src/renderers/dom/client/wrappers/__tests__/ReactDOMTextarea-test.js b/src/renderers/dom/client/wrappers/__tests__/ReactDOMTextarea-test.js
@@ -265,4 +265,17 @@ describe('ReactDOMTextarea', function() {
     renderTextarea(<textarea />, container);
     ReactDOM.unmountComponentAtNode(container);
   });
+
+  it('should throw warning message if value is null', function() {
+    spyOn(console, 'error');
+
+    ReactTestUtils.renderIntoDocument(<textarea value={null} />);
+    expect(console.error.argsForCall[0][0]).toContain(
+       '`value` prop on `textarea` should not be null. Consider using the empty string to clear the component or'
+        + ' `undefined` for uncontrolled components.'
+    );
+
+    ReactTestUtils.renderIntoDocument(<textarea value={null} />);
+    expect(console.error.argsForCall.length).toBe(1);
+  });
 });