Skip to content

Commit 86e8e0a

Browse files
committed
Switch to prop-types & PropTypes.checkPropTypes
PropTypes.checkPropTypes cannot throw yet (see [1]), so checking console.error output [1] facebook/prop-types#54
1 parent fa0f38b commit 86e8e0a

File tree

4 files changed

+24
-28
lines changed

4 files changed

+24
-28
lines changed

README.md

+1-3
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ Use the mixin function which returns a class with PropTypes and defaultProps log
2525
const ValidatingModel = propTypesMixin(Model);
2626
```
2727

28-
If `process.env.NODE_ENV === 'production'`, PropTypes checking will be disabled.
29-
3028
Define your concrete model, and add `propTypes` and `defaultProps` static class attributes.
3129

3230
```javascript
@@ -44,7 +42,7 @@ Person.defaultProps = {
4442
Person.modelName = 'Person';
4543
```
4644

47-
The mixin adds a layer of logic on top of the Model static method `create` and the instance method `update`. When calling `create`, if you have defined `defaultProps`, it'll merge the defaults with the props you passed in. Then, if you've defined `Model.propTypes`, it'll validate the props. An error will be thrown if a prop is found to be invalid. The final props (that may have been merged with defaults) will be passed to the `create` method on the superclass you passed the mixin function.
45+
The mixin adds a layer of logic on top of the Model static method `create` and the instance method `update`. When calling `create`, if you have defined `defaultProps`, it'll merge the defaults with the props you passed in. Then, if you've defined `Model.propTypes`, it'll validate the props. The final props (that may have been merged with defaults) will be passed to the `create` method on the superclass you passed the mixin function.
4846

4947
When you call the `modelInstance.update(attrObj)` instance method, the keys in `attrObj` will be checked against the corresponding `propTypes`, if they exist.
5048

package.json

+4-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
"eslint": "^3.19.0",
3131
"eslint-config-airbnb-base": "^11.2.0",
3232
"eslint-plugin-import": "^2.2.0",
33-
"jest": "^21.2.1",
34-
"react": "^0.14.7"
33+
"jest": "^21.2.1"
34+
},
35+
"dependencies": {
36+
"prop-types": "^15.6.0"
3537
}
3638
}

src/index.js

+5-14
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,4 @@
1-
function validateProp(validator, props, key, modelName) {
2-
const result = validator(props, key, modelName, 'prop');
3-
if (result instanceof Error) {
4-
throw result;
5-
}
6-
}
1+
import { PropTypes } from 'prop-types';
72

83
function hasPropTypes(obj) {
94
return typeof obj.propTypes === 'object';
@@ -13,12 +8,6 @@ function hasDefaultProps(obj) {
138
return typeof obj.defaultProps === 'object';
149
}
1510

16-
function validateProps(props, propTypes, modelName) {
17-
Object.keys(propTypes).forEach((key) => {
18-
validateProp(propTypes[key], props, key, modelName);
19-
});
20-
}
21-
2211
export function getPropTypesMixin(userOpts) {
2312
const opts = userOpts || {};
2413

@@ -48,7 +37,8 @@ export function getPropTypesMixin(userOpts) {
4837
const propsWithDefaults = Object.assign({}, defaults, props);
4938

5039
if (useValidation && hasPropTypes(this)) {
51-
validateProps(propsWithDefaults, this.propTypes, `${this.modelName}.create`);
40+
PropTypes.checkPropTypes(this.propTypes, propsWithDefaults, 'prop',
41+
`${this.modelName}.create`);
5242
}
5343

5444
return super.create(propsWithDefaults, ...rest);
@@ -72,7 +62,8 @@ export function getPropTypesMixin(userOpts) {
7262
}
7363
return result;
7464
}, {});
75-
validateProps(props, propTypesToValidate, `${modelName}.update`);
65+
PropTypes.checkPropTypes(propTypesToValidate, props, 'prop',
66+
`${modelName}.update`);
7667
}
7768

7869
return super.update(...args);

src/index.test.js

+14-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
/* eslint-disable import/no-extraneous-dependencies */
2-
import { PropTypes } from 'react';
1+
import { PropTypes } from 'prop-types';
32

43
import propTypesMixin, { getPropTypesMixin } from './index';
54

@@ -9,6 +8,7 @@ describe('PropTypesMixin', () => {
98
let modelInstance;
109
let createSpy;
1110
let updateSpy;
11+
let consoleErrorSpy;
1212

1313
beforeEach(() => {
1414
Model = class {
@@ -27,6 +27,8 @@ describe('PropTypesMixin', () => {
2727

2828
modelInstance = new ModelWithMixin();
2929
updateSpy = jest.spyOn(modelInstance, 'update');
30+
consoleErrorSpy = jest.spyOn(global.console, 'error');
31+
consoleErrorSpy.mockReset();
3032
});
3133

3234
it('getPropTypesMixin works correctly', () => {
@@ -65,23 +67,26 @@ describe('PropTypesMixin', () => {
6567
};
6668

6769
ModelWithMixin.create({ name: 'shouldnt raise error' });
70+
expect(consoleErrorSpy.mock.calls.length).toBe(0);
6871

69-
const funcShouldThrow = () => ModelWithMixin.create({ notName: 'asd' });
70-
71-
expect(funcShouldThrow).toThrow('ModelWithMixin', 'name');
72+
ModelWithMixin.create({ notName: 'asd' });
73+
expect(consoleErrorSpy.mock.calls.length).toBe(1);
74+
expect(consoleErrorSpy).toBeCalledWith('Warning: Failed prop type: The prop `name` is marked as required in `ModelWithMixin.create`, but its value is `undefined`.');
7275
});
7376

7477
it('raises validation error on update correctly', () => {
7578
ModelWithMixin.propTypes = {
7679
name: PropTypes.string.isRequired,
7780
age: PropTypes.number.isRequired,
7881
};
82+
expect(consoleErrorSpy.mock.calls.length).toBe(0);
83+
const instance = new ModelWithMixin({ name: 'asd', age: 123 });
84+
expect(consoleErrorSpy.mock.calls.length).toBe(0);
7985

80-
const instance = new ModelWithMixin();
81-
82-
const funcShouldThrow = () => instance.update({ name: 123 });
86+
instance.update({ name: 123 });
8387

84-
expect(funcShouldThrow).toThrow('ModelWithMixin', 'name');
88+
expect(consoleErrorSpy.mock.calls.length).toBe(1);
89+
expect(consoleErrorSpy).toBeCalledWith('Warning: Failed prop type: Invalid prop `name` of type `number` supplied to `ModelWithMixin.update`, expected `string`.');
8590
});
8691

8792
it('correctly uses defaultProps', () => {

0 commit comments

Comments
 (0)