Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add forbid-foreign-prop-types rule #1055

Merged
merged 1 commit into from
Feb 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
* [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition
* [react/forbid-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components
* [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes
* [react/forbid-foreign-prop-types](docs/rules/forbid-foreign-prop-types.md): Forbid foreign propTypes
* [react/no-array-index-key](docs/rules/no-array-index-key.md): Prevent using Array index in `key` props
* [react/no-children-prop](docs/rules/no-children-prop.md): Prevent passing children as props
* [react/no-danger](docs/rules/no-danger.md): Prevent usage of dangerous JSX properties
Expand Down
30 changes: 30 additions & 0 deletions docs/rules/forbid-foreign-prop-types.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Forbid foreign propTypes (forbid-foreign-prop-types)

This rule forbids using another component's prop types unless they are explicitly imported/exported. This allows people who want to use [babel-plugin-transform-react-remove-prop-types](https://github.com/oliviertassinari/babel-plugin-transform-react-remove-prop-types) to remove propTypes from their components in production builds, to do so safely.

In order to ensure that imports are explicitly exported it is recommended to use the ["named" rule in eslint-plugin-import](https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/named.md) in conjunction with this rule.

## Rule Details

This rule checks all objects and ensures that the `propTypes` property is not used.

The following patterns are considered warnings:

```js
import SomeComponent from './SomeComponent';
SomeComponent.propTypes;

var { propTypes } = SomeComponent;

SomeComponent['propTypes'];
```

The following patterns are not considered warnings:

```js
import SomeComponent, {propTypes as someComponentPropTypes} from './SomeComponent';
```

## When not to use

This rule aims to make a certain production optimization, removing prop types, less prone to error. This rule may not be relevant to you if you do not wish to make use of this optimization.
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ var allRules = {
'no-direct-mutation-state': require('./lib/rules/no-direct-mutation-state'),
'forbid-component-props': require('./lib/rules/forbid-component-props'),
'forbid-prop-types': require('./lib/rules/forbid-prop-types'),
'forbid-foreign-prop-types': require('./lib/rules/forbid-foreign-prop-types'),
'prefer-es6-class': require('./lib/rules/prefer-es6-class'),
'jsx-key': require('./lib/rules/jsx-key'),
'no-string-refs': require('./lib/rules/no-string-refs'),
Expand Down
63 changes: 63 additions & 0 deletions lib/rules/forbid-foreign-prop-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* @fileoverview Forbid using another component's propTypes
* @author Ian Christian Myers
*/
'use strict';

var find = require('array.prototype.find');

// ------------------------------------------------------------------------------
// Constants
// ------------------------------------------------------------------------------


// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Forbid using another component\'s propTypes',
category: 'Best Practices',
recommended: false
}
},

create: function(context) {
// --------------------------------------------------------------------------
// Helpers
// --------------------------------------------------------------------------

function isLeftSideOfAssignment(node) {
return node.parent.type === 'AssignmentExpression' && node.parent.left === node;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will a node always have a .parent property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that MemberExpression and ObjectPattern will always at least have a parent of Program.

}

return {
MemberExpression: function(node) {
if (!node.computed && node.property && node.property.type === 'Identifier' &&
node.property.name === 'propTypes' && !isLeftSideOfAssignment(node) ||
node.property && node.property.type === 'Literal' &&
node.property.value === 'propTypes' && !isLeftSideOfAssignment(node)) {
context.report({
node: node.property,
message: 'Using another component\'s propTypes is forbidden'
});
}
},

ObjectPattern: function(node) {
var propTypesNode = find(node.properties, function(property) {
return property.type === 'Property' && property.key.name === 'propTypes';
});

if (propTypesNode) {
context.report({
node: propTypesNode,
message: 'Using another component\'s propTypes is forbidden'
});
}
}
};
}
};
156 changes: 156 additions & 0 deletions tests/lib/rules/forbid-foreign-prop-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/**
* @fileoverview Tests for forbid-foreign-prop-types
*/
'use strict';

// -----------------------------------------------------------------------------
// Requirements
// -----------------------------------------------------------------------------

var rule = require('../../../lib/rules/forbid-foreign-prop-types');
var RuleTester = require('eslint').RuleTester;

var parserOptions = {
ecmaVersion: 6,
sourceType: 'module',
ecmaFeatures: {
jsx: true
}
};

require('babel-eslint');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none of the code in this test requires object rest/spread or babel-eslint; could these tests just stick with espree?


// -----------------------------------------------------------------------------
// Tests
// -----------------------------------------------------------------------------

var ERROR_MESSAGE = 'Using another component\'s propTypes is forbidden';

var ruleTester = new RuleTester();
ruleTester.run('forbid-foreign-prop-types', rule, {

valid: [{
code: 'import { propTypes } from "SomeComponent";',
parserOptions: parserOptions
}, {
code: 'import { propTypes as someComponentPropTypes } from "SomeComponent";',
parserOptions: parserOptions
}, {
code: 'const foo = propTypes',
parserOptions: parserOptions
}, {
code: 'foo(propTypes)',
parserOptions: parserOptions
}, {
code: 'foo + propTypes',
parserOptions: parserOptions
}, {
code: 'const foo = [propTypes]',
parserOptions: parserOptions
}, {
code: 'const foo = { propTypes }',
parserOptions: parserOptions
}, {
code: 'Foo.propTypes = propTypes',
parserOptions: parserOptions
}, {
code: 'Foo["propTypes"] = propTypes',
parserOptions: parserOptions
}, {
code: 'const propTypes = "bar"; Foo[propTypes];',
parserOptions: parserOptions
}],

invalid: [{
code: [
'var Foo = React.createClass({',
' propTypes: Bar.propTypes,',
' render: function() {',
' return <Foo className="bar" />;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: ERROR_MESSAGE,
type: 'Identifier'
}]
},
{
code: [
'var Foo = React.createClass({',
' propTypes: Bar["propTypes"],',
' render: function() {',
' return <Foo className="bar" />;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: ERROR_MESSAGE,
type: 'Literal'
}]
},
{
code: [
'var { propTypes } = SomeComponent',
'var Foo = React.createClass({',
' propTypes,',
' render: function() {',
' return <Foo className="bar" />;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: ERROR_MESSAGE,
type: 'Property'
}]
},
{
code: [
'var { propTypes: things, ...foo } = SomeComponent',
'var Foo = React.createClass({',
' propTypes,',
' render: function() {',
' return <Foo className="bar" />;',
' }',
'});'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: ERROR_MESSAGE,
type: 'Property'
}]
},
{
code: [
'class MyComponent extends React.Component {',
' static fooBar = {',
' baz: Qux.propTypes.baz',
' };',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: ERROR_MESSAGE,
type: 'Identifier'
}]
},
{
code: [
'var { propTypes: typesOfProps } = SomeComponent',
'var Foo = React.createClass({',
' propTypes: typesOfProps,',
' render: function() {',
' return <Foo className="bar" />;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: ERROR_MESSAGE,
type: 'Property'
}]
}]
});