PropTypes: distinguish nullable from optional object field (facebook#7291)

* PropTypes: distinguish nullable from optional object field

This gives a more precise message (no type semantics change) to the case of passing a field in an object, but whose value is `null`:

Before:

```js
propTypes: {
  foo: React.PropTypes.number.isRequired
}
```

Would scream "Required prop `foo` was not specified in `MyComp`".

Now it'll be "Required prop `foo` was specified in `MyComp`, but its value is `null`.".

Works as expected in nested objects.

This fixes the issue of a component transitively passing a `null`, specifying the correct field to the child but have the child tell it that it didn't provide the prop.

Optional field and nullable are two different things anyway.

* Add missing test case.

* Reword messages.

Author: chenglou

src/addons/link/__tests__/ReactLinkPropTypes-test.js

@@ -18,8 +18,8 @@ var ReactPropTypeLocations = require('ReactPropTypeLocations');
 var ReactPropTypesSecret = require('ReactPropTypesSecret');
 
 var invalidMessage = 'Invalid prop `testProp` supplied to `testComponent`.';
-var requiredMessage =
-  'Required prop `testProp` was not specified in `testComponent`.';
+var requiredMessage = 'The prop `testProp` is marked as required in ' +
+  '`testComponent`, but its value is `undefined`.';
 
 function typeCheckFail(declaration, value, message) {
   var props = {testProp: value};
@@ -53,22 +53,26 @@ describe('ReactLink', function() {
     typeCheckFail(
       LinkPropTypes.link(React.PropTypes.any),
       {},
-      'Required prop `testProp.value` was not specified in `testComponent`.'
+      'The prop `testProp.value` is marked as required in `testComponent`, ' +
+        'but its value is `undefined`.'
     );
     typeCheckFail(
       LinkPropTypes.link(React.PropTypes.any),
       {value: 123},
-      'Required prop `testProp.requestChange` was not specified in `testComponent`.'
+      'The prop `testProp.requestChange` is marked as required in ' +
+        '`testComponent`, but its value is `undefined`.'
     );
     typeCheckFail(
       LinkPropTypes.link(React.PropTypes.any),
       {requestChange: emptyFunction},
-      'Required prop `testProp.value` was not specified in `testComponent`.'
+      'The prop `testProp.value` is marked as required in `testComponent`, ' +
+        'but its value is `undefined`.'
     );
     typeCheckFail(
       LinkPropTypes.link(React.PropTypes.any),
       {value: null, requestChange: null},
-      'Required prop `testProp.value` was not specified in `testComponent`.'
+      'The prop `testProp.value` is marked as required in `testComponent`, ' +
+        'but its value is `null`.'
     );
   });
 
@@ -122,12 +126,14 @@ describe('ReactLink', function() {
   });
 
   it('should warn for missing required values', function() {
-    typeCheckFail(LinkPropTypes.link().isRequired, null, requiredMessage);
+    var specifiedButIsNullMsg = 'The prop `testProp` is marked as required ' +
+      'in `testComponent`, but its value is `null`.';
+    typeCheckFail(LinkPropTypes.link().isRequired, null, specifiedButIsNullMsg);
     typeCheckFail(LinkPropTypes.link().isRequired, undefined, requiredMessage);
     typeCheckFail(
       LinkPropTypes.link(React.PropTypes.string).isRequired,
       null,
-      requiredMessage
+      specifiedButIsNullMsg
     );
     typeCheckFail(
       LinkPropTypes.link(React.PropTypes.string).isRequired,

src/isomorphic/classic/__tests__/ReactContextValidator-test.js

@@ -152,7 +152,8 @@ describe('ReactContextValidator', function() {
     expect(console.error.calls.count()).toBe(1);
     expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
       'Warning: Failed context type: ' +
-      'Required context `foo` was not specified in `Component`.\n' +
+      'The context `foo` is marked as required in `Component`, but its value ' +
+      'is `undefined`.\n' +
       '    in Component (at **)'
     );
 
@@ -229,7 +230,8 @@ describe('ReactContextValidator', function() {
     expect(console.error.calls.count()).toBe(1);
     expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
       'Warning: Failed childContext type: ' +
-      'Required child context `foo` was not specified in `Component`.\n' +
+      'The child context `foo` is marked as required in `Component`, but its ' +
+      'value is `undefined`.\n' +
       '    in Component (at **)'
     );
 

src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js

@@ -360,8 +360,8 @@ describe('ReactElementValidator', function() {
 
     expect(console.error.calls.count()).toBe(1);
     expect(console.error.calls.argsFor(0)[0]).toBe(
-      'Warning: Failed prop type: ' +
-      'Required prop `prop` was not specified in `Component`.\n' +
+      'Warning: Failed prop type: The prop `prop` is marked as required in ' +
+      '`Component`, but its value is `null`.\n' +
       '    in Component'
     );
   });
@@ -385,8 +385,8 @@ describe('ReactElementValidator', function() {
 
     expect(console.error.calls.count()).toBe(1);
     expect(console.error.calls.argsFor(0)[0]).toBe(
-      'Warning: Failed prop type: ' +
-      'Required prop `prop` was not specified in `Component`.\n' +
+      'Warning: Failed prop type: The prop `prop` is marked as required in ' +
+      '`Component`, but its value is `null`.\n' +
       '    in Component'
     );
   });
@@ -413,7 +413,8 @@ describe('ReactElementValidator', function() {
     expect(console.error.calls.count()).toBe(2);
     expect(console.error.calls.argsFor(0)[0]).toBe(
       'Warning: Failed prop type: ' +
-      'Required prop `prop` was not specified in `Component`.\n' +
+      'The prop `prop` is marked as required in `Component`, but its value ' +
+      'is `undefined`.\n' +
       '    in Component'
     );
 

src/isomorphic/classic/types/ReactPropTypes.js

@@ -144,9 +144,15 @@ function createChainableTypeChecker(validate) {
     if (props[propName] == null) {
       var locationName = ReactPropTypeLocationNames[location];
       if (isRequired) {
+        if (props[propName] === null) {
+          return new Error(
+            `The ${locationName} \`${propFullName}\` is marked as required ` +
+            `in \`${componentName}\`, but its value is \`null\`.`
+          );
+        }
         return new Error(
-          `Required ${locationName} \`${propFullName}\` was not specified in ` +
-          `\`${componentName}\`.`
+          `The ${locationName} \`${propFullName}\` is marked as required in ` +
+          `\`${componentName}\`, but its value is \`undefined\`.`
         );
       }
       return null;

src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js

@@ -20,8 +20,6 @@ var ReactPropTypesSecret;
 
 var Component;
 var MyComponent;
-var requiredMessage =
-  'Required prop `testProp` was not specified in `testComponent`.';
 
 function typeCheckFail(declaration, value, message) {
   var props = {testProp: value};
@@ -37,6 +35,46 @@ function typeCheckFail(declaration, value, message) {
   expect(error.message).toBe(message);
 }
 
+function typeCheckFailRequiredValues(declaration) {
+  var specifiedButIsNullMsg = 'The prop `testProp` is marked as required in ' +
+    '`testComponent`, but its value is `null`.';
+  var unspecifiedMsg = 'The prop `testProp` is marked as required in ' +
+    '`testComponent`, but its value is \`undefined\`.';
+  var props1 = {testProp: null};
+  var error1 = declaration(
+    props1,
+    'testProp',
+    'testComponent',
+    ReactPropTypeLocations.prop,
+    null,
+    ReactPropTypesSecret
+  );
+  expect(error1 instanceof Error).toBe(true);
+  expect(error1.message).toBe(specifiedButIsNullMsg);
+  var props2 = {testProp: undefined};
+  var error2 = declaration(
+    props2,
+    'testProp',
+    'testComponent',
+    ReactPropTypeLocations.prop,
+    null,
+    ReactPropTypesSecret
+  );
+  expect(error2 instanceof Error).toBe(true);
+  expect(error2.message).toBe(unspecifiedMsg);
+  var props3 = {};
+  var error3 = declaration(
+    props3,
+    'testProp',
+    'testComponent',
+    ReactPropTypeLocations.prop,
+    null,
+    ReactPropTypesSecret
+  );
+  expect(error3 instanceof Error).toBe(true);
+  expect(error3.message).toBe(unspecifiedMsg);
+}
+
 function typeCheckPass(declaration, value) {
   var props = {testProp: value};
   var error = declaration(
@@ -146,8 +184,7 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(PropTypes.string.isRequired, null, requiredMessage);
-      typeCheckFail(PropTypes.string.isRequired, undefined, requiredMessage);
+      typeCheckFailRequiredValues(PropTypes.string.isRequired);
     });
 
     it('should warn if called manually in development', function() {
@@ -213,8 +250,7 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(PropTypes.any.isRequired, null, requiredMessage);
-      typeCheckFail(PropTypes.any.isRequired, undefined, requiredMessage);
+      typeCheckFailRequiredValues(PropTypes.any.isRequired);
     });
 
     it('should warn if called manually in development', function() {
@@ -307,15 +343,8 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.arrayOf(PropTypes.number).isRequired,
-        null,
-        requiredMessage
-      );
-      typeCheckFail(
-        PropTypes.arrayOf(PropTypes.number).isRequired,
-        undefined,
-        requiredMessage
+      typeCheckFailRequiredValues(
+        PropTypes.arrayOf(PropTypes.number).isRequired
       );
     });
 
@@ -406,8 +435,7 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(PropTypes.element.isRequired, null, requiredMessage);
-      typeCheckFail(PropTypes.element.isRequired, undefined, requiredMessage);
+      typeCheckFailRequiredValues(PropTypes.element.isRequired);
     });
 
     it('should warn if called manually in development', function() {
@@ -493,12 +521,7 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.instanceOf(String).isRequired, null, requiredMessage
-      );
-      typeCheckFail(
-        PropTypes.instanceOf(String).isRequired, undefined, requiredMessage
-      );
+      typeCheckFailRequiredValues(PropTypes.instanceOf(String).isRequired);
     });
 
     it('should warn if called manually in development', function() {
@@ -601,16 +624,7 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.node.isRequired,
-        null,
-        'Required prop `testProp` was not specified in `testComponent`.'
-      );
-      typeCheckFail(
-        PropTypes.node.isRequired,
-        undefined,
-        'Required prop `testProp` was not specified in `testComponent`.'
-      );
+      typeCheckFailRequiredValues(PropTypes.node.isRequired);
     });
 
     it('should accept empty array for required props', function() {
@@ -724,15 +738,8 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.objectOf(PropTypes.number).isRequired,
-        null,
-        requiredMessage
-      );
-      typeCheckFail(
-        PropTypes.objectOf(PropTypes.number).isRequired,
-        undefined,
-        requiredMessage
+      typeCheckFailRequiredValues(
+        PropTypes.objectOf(PropTypes.number).isRequired
       );
     });
 
@@ -804,16 +811,7 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.oneOf(['red', 'blue']).isRequired,
-        null,
-        requiredMessage
-      );
-      typeCheckFail(
-        PropTypes.oneOf(['red', 'blue']).isRequired,
-        undefined,
-        requiredMessage
-      );
+      typeCheckFailRequiredValues(PropTypes.oneOf(['red', 'blue']).isRequired);
     });
 
     it('should warn if called manually in development', function() {
@@ -882,15 +880,8 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,
-        null,
-        requiredMessage
-      );
-      typeCheckFail(
-        PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,
-        undefined,
-        requiredMessage
+      typeCheckFailRequiredValues(
+        PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired
       );
     });
 
@@ -950,7 +941,8 @@ describe('ReactPropTypes', function() {
       typeCheckFail(
         PropTypes.shape({key: PropTypes.number.isRequired}),
         {},
-        'Required prop `testProp.key` was not specified in `testComponent`.'
+        'The prop `testProp.key` is marked as required in `testComponent`, ' +
+          'but its value is `undefined`.'
       );
     });
 
@@ -961,7 +953,8 @@ describe('ReactPropTypes', function() {
           secondKey: PropTypes.number.isRequired,
         }),
         {},
-        'Required prop `testProp.key` was not specified in `testComponent`.'
+        'The prop `testProp.key` is marked as required in `testComponent`, ' +
+          'but its value is `undefined`.'
       );
     });
 
@@ -983,15 +976,8 @@ describe('ReactPropTypes', function() {
     });
 
     it('should warn for missing required values', function() {
-      typeCheckFail(
-        PropTypes.shape({key: PropTypes.number}).isRequired,
-        null,
-        requiredMessage
-      );
-      typeCheckFail(
-        PropTypes.shape({key: PropTypes.number}).isRequired,
-        undefined,
-        requiredMessage
+      typeCheckFailRequiredValues(
+        PropTypes.shape({key: PropTypes.number}).isRequired
       );
     });
 

src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js

@@ -249,8 +249,8 @@ describe('ReactJSXElementValidator', function() {
     expect(
       console.error.calls.argsFor(0)[0].replace(/\(at .+?:\d+\)/g, '(at **)')
     ).toBe(
-      'Warning: Failed prop type: ' +
-      'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +
+      'Warning: Failed prop type: The prop `prop` is marked as required in ' +
+      '`RequiredPropComponent`, but its value is `null`.\n' +
       '    in RequiredPropComponent (at **)'
     );
   });
@@ -264,8 +264,8 @@ describe('ReactJSXElementValidator', function() {
     expect(
       console.error.calls.argsFor(0)[0].replace(/\(at .+?:\d+\)/g, '(at **)')
     ).toBe(
-      'Warning: Failed prop type: ' +
-      'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +
+      'Warning: Failed prop type: The prop `prop` is marked as required in ' +
+      '`RequiredPropComponent`, but its value is `null`.\n' +
       '    in RequiredPropComponent (at **)'
     );
   });
@@ -281,7 +281,8 @@ describe('ReactJSXElementValidator', function() {
       console.error.calls.argsFor(0)[0].replace(/\(at .+?:\d+\)/g, '(at **)')
     ).toBe(
       'Warning: Failed prop type: ' +
-      'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +
+      'The prop `prop` is marked as required in `RequiredPropComponent`, but ' +
+      'its value is `undefined`.\n' +
       '    in RequiredPropComponent (at **)'
     );
 

src/test/__tests__/ReactTestUtils-test.js

@@ -296,8 +296,8 @@ describe('ReactTestUtils', function() {
     expect(
       console.error.calls.argsFor(0)[0].replace(/\(at .+?:\d+\)/g, '(at **)')
     ).toBe(
-      'Warning: Failed context type: Required context `name` was not ' +
-      'specified in `SimpleComponent`.\n' +
+      'Warning: Failed context type: The context `name` is marked as ' +
+      'required in `SimpleComponent`, but its value is `undefined`.\n' +
       '    in SimpleComponent (at **)'
     );
   });