@samitouri / QOS-React / commits / 0292d34047

PropTypes: distinguish nullable from optional object field (#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.

Cheng Lou committed Jul 26, 2016 at 16:26 UTC 0292d3404787b08b19267a3c951170a0dabd0e98

7 files changed +98 -96

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

+14 -8

@@ -18,8 +18,8 @@ var ReactPropTypeLocations = require('ReactPropTypeLocations');

18 var ReactPropTypesSecret = require('ReactPropTypesSecret');

20 var invalidMessage = 'Invalid prop `testProp` supplied to `testComponent`.';

21 -var requiredMessage =

22 - 'Required prop `testProp` was not specified in `testComponent`.';

21 +var requiredMessage = 'The prop `testProp` is marked as required in ' +

22 + '`testComponent`, but its value is `undefined`.';

24 function typeCheckFail(declaration, value, message) {

25 var props = {testProp: value};

@@ -53,22 +53,26 @@ describe('ReactLink', function() {

53 typeCheckFail(

54 LinkPropTypes.link(React.PropTypes.any),

55 {},

56 - 'Required prop `testProp.value` was not specified in `testComponent`.'

56 + 'The prop `testProp.value` is marked as required in `testComponent`, ' +

57 + 'but its value is `undefined`.'

58 );

59 typeCheckFail(

60 LinkPropTypes.link(React.PropTypes.any),

61 {value: 123},

61 - 'Required prop `testProp.requestChange` was not specified in `testComponent`.'

62 + 'The prop `testProp.requestChange` is marked as required in ' +

63 + '`testComponent`, but its value is `undefined`.'

64 );

65 typeCheckFail(

66 LinkPropTypes.link(React.PropTypes.any),

67 {requestChange: emptyFunction},

66 - 'Required prop `testProp.value` was not specified in `testComponent`.'

68 + 'The prop `testProp.value` is marked as required in `testComponent`, ' +

69 + 'but its value is `undefined`.'

70 );

71 typeCheckFail(

72 LinkPropTypes.link(React.PropTypes.any),

73 {value: null, requestChange: null},

71 - 'Required prop `testProp.value` was not specified in `testComponent`.'

74 + 'The prop `testProp.value` is marked as required in `testComponent`, ' +

75 + 'but its value is `null`.'

76 );

77 });

@@ -122,12 +126,14 @@ describe('ReactLink', function() {

126 });

127

128 it('should warn for missing required values', function() {

125 - typeCheckFail(LinkPropTypes.link().isRequired, null, requiredMessage);

129 + var specifiedButIsNullMsg = 'The prop `testProp` is marked as required ' +

130 + 'in `testComponent`, but its value is `null`.';

131 + typeCheckFail(LinkPropTypes.link().isRequired, null, specifiedButIsNullMsg);

132 typeCheckFail(LinkPropTypes.link().isRequired, undefined, requiredMessage);

133 typeCheckFail(

134 LinkPropTypes.link(React.PropTypes.string).isRequired,

135 null,

130 - requiredMessage

136 + specifiedButIsNullMsg

137 );

138 typeCheckFail(

139 LinkPropTypes.link(React.PropTypes.string).isRequired,

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

+4 -2

@@ -152,7 +152,8 @@ describe('ReactContextValidator', function() {

152 expect(console.error.calls.count()).toBe(1);

153 expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(

154 'Warning: Failed context type: ' +

155 - 'Required context `foo` was not specified in `Component`.\n' +

155 + 'The context `foo` is marked as required in `Component`, but its value ' +

156 + 'is `undefined`.\n' +

157 ' in Component (at **)'

158 );

159

@@ -229,7 +230,8 @@ describe('ReactContextValidator', function() {

230 expect(console.error.calls.count()).toBe(1);

231 expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(

232 'Warning: Failed childContext type: ' +

232 - 'Required child context `foo` was not specified in `Component`.\n' +

233 + 'The child context `foo` is marked as required in `Component`, but its ' +

234 + 'value is `undefined`.\n' +

235 ' in Component (at **)'

236 );

237

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

+6 -5

@@ -360,8 +360,8 @@ describe('ReactElementValidator', function() {

360

361 expect(console.error.calls.count()).toBe(1);

362 expect(console.error.calls.argsFor(0)[0]).toBe(

363 - 'Warning: Failed prop type: ' +

364 - 'Required prop `prop` was not specified in `Component`.\n' +

363 + 'Warning: Failed prop type: The prop `prop` is marked as required in ' +

364 + '`Component`, but its value is `null`.\n' +

365 ' in Component'

366 );

367 });

@@ -385,8 +385,8 @@ describe('ReactElementValidator', function() {

385

386 expect(console.error.calls.count()).toBe(1);

387 expect(console.error.calls.argsFor(0)[0]).toBe(

388 - 'Warning: Failed prop type: ' +

389 - 'Required prop `prop` was not specified in `Component`.\n' +

388 + 'Warning: Failed prop type: The prop `prop` is marked as required in ' +

389 + '`Component`, but its value is `null`.\n' +

390 ' in Component'

391 );

392 });

@@ -413,7 +413,8 @@ describe('ReactElementValidator', function() {

413 expect(console.error.calls.count()).toBe(2);

414 expect(console.error.calls.argsFor(0)[0]).toBe(

415 'Warning: Failed prop type: ' +

416 - 'Required prop `prop` was not specified in `Component`.\n' +

416 + 'The prop `prop` is marked as required in `Component`, but its value ' +

417 + 'is `undefined`.\n' +

418 ' in Component'

419 );

420

src/isomorphic/classic/types/ReactPropTypes.js

+8 -2

@@ -144,9 +144,15 @@ function createChainableTypeChecker(validate) {

144 if (props[propName] == null) {

145 var locationName = ReactPropTypeLocationNames[location];

146 if (isRequired) {

147 + if (props[propName] === null) {

148 + return new Error(

149 + `The ${locationName} \`${propFullName}\` is marked as required ` +

150 + `in \`${componentName}\`, but its value is \`null\`.`

151 + );

152 + }

153 return new Error(

148 - `Required ${locationName} \`${propFullName}\` was not specified in ` +

149 - `\`${componentName}\`.`

154 + `The ${locationName} \`${propFullName}\` is marked as required in ` +

155 + `\`${componentName}\`, but its value is \`undefined\`.`

156 );

157 }

158 return null;

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

+58 -72

@@ -20,8 +20,6 @@ var ReactPropTypesSecret;

21 var Component;

22 var MyComponent;

23 -var requiredMessage =

24 - 'Required prop `testProp` was not specified in `testComponent`.';

24 function typeCheckFail(declaration, value, message) {

25 var props = {testProp: value};

@@ -37,6 +35,46 @@ function typeCheckFail(declaration, value, message) {

35 expect(error.message).toBe(message);

36 }

38 +function typeCheckFailRequiredValues(declaration) {

39 + var specifiedButIsNullMsg = 'The prop `testProp` is marked as required in ' +

40 + '`testComponent`, but its value is `null`.';

41 + var unspecifiedMsg = 'The prop `testProp` is marked as required in ' +

42 + '`testComponent`, but its value is \`undefined\`.';

43 + var props1 = {testProp: null};

44 + var error1 = declaration(

45 + props1,

46 + 'testProp',

47 + 'testComponent',

48 + ReactPropTypeLocations.prop,

49 + null,

50 + ReactPropTypesSecret

51 + );

52 + expect(error1 instanceof Error).toBe(true);

53 + expect(error1.message).toBe(specifiedButIsNullMsg);

54 + var props2 = {testProp: undefined};

55 + var error2 = declaration(

56 + props2,

57 + 'testProp',

58 + 'testComponent',

59 + ReactPropTypeLocations.prop,

60 + null,

61 + ReactPropTypesSecret

62 + );

63 + expect(error2 instanceof Error).toBe(true);

64 + expect(error2.message).toBe(unspecifiedMsg);

65 + var props3 = {};

66 + var error3 = declaration(

67 + props3,

68 + 'testProp',

69 + 'testComponent',

70 + ReactPropTypeLocations.prop,

71 + null,

72 + ReactPropTypesSecret

73 + );

74 + expect(error3 instanceof Error).toBe(true);

75 + expect(error3.message).toBe(unspecifiedMsg);

76 +}

77 +

78 function typeCheckPass(declaration, value) {

79 var props = {testProp: value};

80 var error = declaration(

@@ -146,8 +184,7 @@ describe('ReactPropTypes', function() {

184 });

185

186 it('should warn for missing required values', function() {

149 - typeCheckFail(PropTypes.string.isRequired, null, requiredMessage);

150 - typeCheckFail(PropTypes.string.isRequired, undefined, requiredMessage);

187 + typeCheckFailRequiredValues(PropTypes.string.isRequired);

188 });

189

190 it('should warn if called manually in development', function() {

@@ -213,8 +250,7 @@ describe('ReactPropTypes', function() {

250 });

251

252 it('should warn for missing required values', function() {

216 - typeCheckFail(PropTypes.any.isRequired, null, requiredMessage);

217 - typeCheckFail(PropTypes.any.isRequired, undefined, requiredMessage);

253 + typeCheckFailRequiredValues(PropTypes.any.isRequired);

254 });

255

256 it('should warn if called manually in development', function() {

@@ -307,15 +343,8 @@ describe('ReactPropTypes', function() {

343 });

344

345 it('should warn for missing required values', function() {

310 - typeCheckFail(

311 - PropTypes.arrayOf(PropTypes.number).isRequired,

312 - null,

313 - requiredMessage

314 - );

315 - typeCheckFail(

316 - PropTypes.arrayOf(PropTypes.number).isRequired,

317 - undefined,

318 - requiredMessage

346 + typeCheckFailRequiredValues(

347 + PropTypes.arrayOf(PropTypes.number).isRequired

348 );

349 });

350

@@ -406,8 +435,7 @@ describe('ReactPropTypes', function() {

435 });

436

437 it('should warn for missing required values', function() {

409 - typeCheckFail(PropTypes.element.isRequired, null, requiredMessage);

410 - typeCheckFail(PropTypes.element.isRequired, undefined, requiredMessage);

438 + typeCheckFailRequiredValues(PropTypes.element.isRequired);

439 });

440

441 it('should warn if called manually in development', function() {

@@ -493,12 +521,7 @@ describe('ReactPropTypes', function() {

521 });

522

523 it('should warn for missing required values', function() {

496 - typeCheckFail(

497 - PropTypes.instanceOf(String).isRequired, null, requiredMessage

498 - );

499 - typeCheckFail(

500 - PropTypes.instanceOf(String).isRequired, undefined, requiredMessage

501 - );

524 + typeCheckFailRequiredValues(PropTypes.instanceOf(String).isRequired);

525 });

526

527 it('should warn if called manually in development', function() {

@@ -601,16 +624,7 @@ describe('ReactPropTypes', function() {

624 });

625

626 it('should warn for missing required values', function() {

604 - typeCheckFail(

605 - PropTypes.node.isRequired,

606 - null,

607 - 'Required prop `testProp` was not specified in `testComponent`.'

608 - );

609 - typeCheckFail(

610 - PropTypes.node.isRequired,

611 - undefined,

612 - 'Required prop `testProp` was not specified in `testComponent`.'

613 - );

627 + typeCheckFailRequiredValues(PropTypes.node.isRequired);

628 });

629

630 it('should accept empty array for required props', function() {

@@ -724,15 +738,8 @@ describe('ReactPropTypes', function() {

738 });

739

740 it('should warn for missing required values', function() {

727 - typeCheckFail(

728 - PropTypes.objectOf(PropTypes.number).isRequired,

729 - null,

730 - requiredMessage

731 - );

732 - typeCheckFail(

733 - PropTypes.objectOf(PropTypes.number).isRequired,

734 - undefined,

735 - requiredMessage

741 + typeCheckFailRequiredValues(

742 + PropTypes.objectOf(PropTypes.number).isRequired

743 );

744 });

745

@@ -804,16 +811,7 @@ describe('ReactPropTypes', function() {

811 });

812

813 it('should warn for missing required values', function() {

807 - typeCheckFail(

808 - PropTypes.oneOf(['red', 'blue']).isRequired,

809 - null,

810 - requiredMessage

811 - );

812 - typeCheckFail(

813 - PropTypes.oneOf(['red', 'blue']).isRequired,

814 - undefined,

815 - requiredMessage

816 - );

814 + typeCheckFailRequiredValues(PropTypes.oneOf(['red', 'blue']).isRequired);

815 });

816

817 it('should warn if called manually in development', function() {

@@ -882,15 +880,8 @@ describe('ReactPropTypes', function() {

880 });

881

882 it('should warn for missing required values', function() {

885 - typeCheckFail(

886 - PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,

887 - null,

888 - requiredMessage

889 - );

890 - typeCheckFail(

891 - PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,

892 - undefined,

893 - requiredMessage

883 + typeCheckFailRequiredValues(

884 + PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired

885 );

886 });

887

@@ -950,7 +941,8 @@ describe('ReactPropTypes', function() {

941 typeCheckFail(

942 PropTypes.shape({key: PropTypes.number.isRequired}),

943 {},

953 - 'Required prop `testProp.key` was not specified in `testComponent`.'

944 + 'The prop `testProp.key` is marked as required in `testComponent`, ' +

945 + 'but its value is `undefined`.'

946 );

947 });

948

@@ -961,7 +953,8 @@ describe('ReactPropTypes', function() {

953 secondKey: PropTypes.number.isRequired,

954 }),

955 {},

964 - 'Required prop `testProp.key` was not specified in `testComponent`.'

956 + 'The prop `testProp.key` is marked as required in `testComponent`, ' +

957 + 'but its value is `undefined`.'

958 );

959 });

960

@@ -983,15 +976,8 @@ describe('ReactPropTypes', function() {

976 });

977

978 it('should warn for missing required values', function() {

986 - typeCheckFail(

987 - PropTypes.shape({key: PropTypes.number}).isRequired,

988 - null,

989 - requiredMessage

990 - );

991 - typeCheckFail(

992 - PropTypes.shape({key: PropTypes.number}).isRequired,

993 - undefined,

994 - requiredMessage

979 + typeCheckFailRequiredValues(

980 + PropTypes.shape({key: PropTypes.number}).isRequired

981 );

982 });

983

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

+6 -5

@@ -249,8 +249,8 @@ describe('ReactJSXElementValidator', function() {

249 expect(

250 console.error.calls.argsFor(0)[0].replace(/$at .+?:\d+$/g, '(at **)')

251 ).toBe(

252 - 'Warning: Failed prop type: ' +

253 - 'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +

252 + 'Warning: Failed prop type: The prop `prop` is marked as required in ' +

253 + '`RequiredPropComponent`, but its value is `null`.\n' +

254 ' in RequiredPropComponent (at **)'

255 );

256 });

@@ -264,8 +264,8 @@ describe('ReactJSXElementValidator', function() {

264 expect(

265 console.error.calls.argsFor(0)[0].replace(/$at .+?:\d+$/g, '(at **)')

266 ).toBe(

267 - 'Warning: Failed prop type: ' +

268 - 'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +

267 + 'Warning: Failed prop type: The prop `prop` is marked as required in ' +

268 + '`RequiredPropComponent`, but its value is `null`.\n' +

269 ' in RequiredPropComponent (at **)'

270 );

271 });

@@ -281,7 +281,8 @@ describe('ReactJSXElementValidator', function() {

281 console.error.calls.argsFor(0)[0].replace(/$at .+?:\d+$/g, '(at **)')

282 ).toBe(

283 'Warning: Failed prop type: ' +

284 - 'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +

284 + 'The prop `prop` is marked as required in `RequiredPropComponent`, but ' +

285 + 'its value is `undefined`.\n' +

286 ' in RequiredPropComponent (at **)'

287 );

288

src/test/__tests__/ReactTestUtils-test.js

+2 -2

@@ -296,8 +296,8 @@ describe('ReactTestUtils', function() {

296 expect(

297 console.error.calls.argsFor(0)[0].replace(/$at .+?:\d+$/g, '(at **)')

298 ).toBe(

299 - 'Warning: Failed context type: Required context `name` was not ' +

300 - 'specified in `SimpleComponent`.\n' +

299 + 'Warning: Failed context type: The context `name` is marked as ' +

300 + 'required in `SimpleComponent`, but its value is `undefined`.\n' +

301 ' in SimpleComponent (at **)'

302 );

303 });