Skip to content

Commit

Permalink
Switch to prop-types & PropTypes.checkPropTypes
Browse files Browse the repository at this point in the history
PropTypes.checkPropTypes cannot throw yet (see [1]), so checking
console.error output

[1] facebook/prop-types#54
  • Loading branch information
ojab committed Oct 3, 2017
1 parent 954e053 commit 66d25c2
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 28 deletions.
4 changes: 1 addition & 3 deletions README.md
Expand Up @@ -25,8 +25,6 @@ Use the mixin function which returns a class with PropTypes and defaultProps log
const ValidatingModel = propTypesMixin(Model);
```

If `process.env.NODE_ENV === 'production'`, PropTypes checking will be disabled.

Define your concrete model, and add `propTypes` and `defaultProps` static class attributes.

```javascript
Expand All @@ -44,7 +42,7 @@ Person.defaultProps = {
Person.modelName = 'Person';
```

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.
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.

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

Expand Down
6 changes: 4 additions & 2 deletions package.json
Expand Up @@ -30,7 +30,9 @@
"eslint": "^3.19.0",
"eslint-config-airbnb-base": "^11.2.0",
"eslint-plugin-import": "^2.2.0",
"jest": "^21.2.1",
"react": "^0.14.7"
"jest": "^21.2.1"
},
"dependencies": {
"prop-types": "^15.6.0"
}
}
19 changes: 5 additions & 14 deletions src/index.js
@@ -1,9 +1,4 @@
function validateProp(validator, props, key, modelName) {
const result = validator(props, key, modelName, 'prop');
if (result instanceof Error) {
throw result;
}
}
import { PropTypes } from 'prop-types';

function hasPropTypes(obj) {
return typeof obj.propTypes === 'object';
Expand All @@ -13,12 +8,6 @@ function hasDefaultProps(obj) {
return typeof obj.defaultProps === 'object';
}

function validateProps(props, propTypes, modelName) {
Object.keys(propTypes).forEach((key) => {
validateProp(propTypes[key], props, key, modelName);
});
}

export function getPropTypesMixin(userOpts) {
const opts = userOpts || {};

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

if (useValidation && hasPropTypes(this)) {
validateProps(propsWithDefaults, this.propTypes, `${this.modelName}.create`);
PropTypes.checkPropTypes(this.propTypes, propsWithDefaults, 'prop',
`${this.modelName}.create`);
}

return super.create(propsWithDefaults, ...rest);
Expand All @@ -72,7 +62,8 @@ export function getPropTypesMixin(userOpts) {
}
return result;
}, {});
validateProps(props, propTypesToValidate, `${modelName}.update`);
PropTypes.checkPropTypes(propTypesToValidate, props, 'prop',
`${modelName}.update`);
}

return super.update(...args);
Expand Down
23 changes: 14 additions & 9 deletions src/index.test.js
@@ -1,5 +1,4 @@
/* eslint-disable import/no-extraneous-dependencies */
import { PropTypes } from 'react';
import { PropTypes } from 'prop-types';

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

Expand All @@ -9,6 +8,7 @@ describe('PropTypesMixin', () => {
let modelInstance;
let createSpy;
let updateSpy;
let consoleErrorSpy;

beforeEach(() => {
Model = class {
Expand All @@ -27,6 +27,8 @@ describe('PropTypesMixin', () => {

modelInstance = new ModelWithMixin();
updateSpy = jest.spyOn(modelInstance, 'update');
consoleErrorSpy = jest.spyOn(global.console, 'error');
consoleErrorSpy.mockReset();
});

it('getPropTypesMixin works correctly', () => {
Expand Down Expand Up @@ -65,23 +67,26 @@ describe('PropTypesMixin', () => {
};

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

const funcShouldThrow = () => ModelWithMixin.create({ notName: 'asd' });

expect(funcShouldThrow).toThrow('ModelWithMixin', 'name');
ModelWithMixin.create({ notName: 'asd' });
expect(consoleErrorSpy.mock.calls.length).toBe(1);
expect(consoleErrorSpy).toBeCalledWith('Warning: Failed prop type: The prop `name` is marked as required in `ModelWithMixin.create`, but its value is `undefined`.');
});

it('raises validation error on update correctly', () => {
ModelWithMixin.propTypes = {
name: PropTypes.string.isRequired,
age: PropTypes.number.isRequired,
};
expect(consoleErrorSpy.mock.calls.length).toBe(0);
const instance = new ModelWithMixin({ name: 'asd', age: 123 });
expect(consoleErrorSpy.mock.calls.length).toBe(0);

const instance = new ModelWithMixin();

const funcShouldThrow = () => instance.update({ name: 123 });
instance.update({ name: 123 });

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

it('correctly uses defaultProps', () => {
Expand Down

0 comments on commit 66d25c2

Please sign in to comment.