[PropTypes] Add warnings if PropTypes return functions
Summary: Right now, if a component specifies a propType as, for example, `myProp: React.PropTypes.shape`, without an actual shape parameter, any prop type will be accepted, because `React.PropTypes.shape` returns a function (the actual validator), not an Error, currently indicating that propType checking passed. This can create an unfortunate situation where a component looks like it has fully specified `propTypes`, but in fact does not. This commit addresses this by warning if a propType checker returns anything non-falsy that is not an Error (currently all the library PropTypes return null or an Error). Test Plan: Added a unit test; ran `jest` in the root repo directory. Also ran `grunt lint` and `grunt test`
Aria Buckles committed
Mar 10, 2015 at 15:21 UTC
d973e32336913b8dd5cbc1fb5d7caaed31ea2cf8
3 files changed
+40
-2
src/classic/element/ReactElementValidator.js
+12
@@ -247,6 +247,18 @@ function checkPropTypes(componentName, propTypes, props, location) {
247
} catch (ex) {
248
error = ex;
249
}
250
+ warning(
251
+ !error || error instanceof Error,
252
+ '%s: the type of %s `%s` is invalid; the type checker function ' +
253
+ 'must return `null` or an `Error`, but returned a %s. ' +
254
+ 'You may have forgotten to pass an argument to the type checker ' +
255
+ 'creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, and ' +
256
+ 'shape all require an argument).',
257
+ componentName || 'React class',
258
+ ReactPropTypeLocationNames[location],
259
+ propName,
260
+ typeof error
261
+ );
262
if (error instanceof Error && !(error.message in loggedTypeFailures)) {
263
// Only monitor this failure once because there tends to be a lot of the
264
// same error.
src/classic/element/__tests__/ReactElementValidator-test.js
+26
@@ -362,6 +362,32 @@ describe('ReactElementValidator', function() {
362
expect(console.warn.calls.length).toBe(2);
363
});
364
365
+ it('should warn if a PropType creator is used as a PropType', function() {
366
+ spyOn(console, 'warn');
367
+
368
+ var Component = React.createClass({
369
+ propTypes: {
370
+ prop: React.PropTypes.shape
371
+ },
372
+ render: function() {
373
+ return React.createElement('span', null, this.props.prop.value);
374
+ }
375
+ });
376
+
377
+ ReactTestUtils.renderIntoDocument(
378
+ React.createElement(Component, {prop: {value: 'hi'}})
379
+ );
380
+
381
+ expect(console.warn.calls.length).toBe(1);
382
+ expect(console.warn.calls[0].args[0]).toBe(
383
+ 'Warning: Component: the type of prop `prop` is invalid; the type ' +
384
+ 'checker function must return `null` or an `Error`, but returned a ' +
385
+ 'function. You may have forgotten to pass an argument to the type ' +
386
+ 'checker creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, ' +
387
+ 'and shape all require an argument).'
388
+ );
389
+ });
390
+
391
it('should warn if a fragment is used without the wrapper', function() {
392
spyOn(console, 'warn');
393
var child = React.createElement('span');
src/classic/types/__tests__/ReactPropTypes-test.js
+2
-2
@@ -777,11 +777,11 @@ describe('ReactPropTypes', function() {
777
'Warning: Failed propType: num must be 5!');
778
});
779
780
- it('should not warn if the validator returned anything else than an error',
780
+ it('should not warn if the validator returned null',
781
function() {
782
var spy = jasmine.createSpy().andCallFake(
783
function(props, propName, componentName) {
784
- return 'This message will never reach anyone';
784
+ return null;
785
}
786
);
787
var Component = React.createClass({