From 66e8e24e737fcea201c316cd0be4ab5dce776d17 Mon Sep 17 00:00:00 2001 From: Ian Christian Myers Date: Tue, 31 Jan 2017 11:12:47 -0800 Subject: [PATCH] Add forbid-foreign-prop-types rule People may want to use babel-plugin-transform-react-remove-prop-types to remove propTypes from their components in production builds, as an optimization. The `forbib-foreign-prop-types` rule forbids using another component's prop types unless they are explicitly imported/exported, which makes that optimization less prone to error. Fixes #696 --- README.md | 1 + docs/rules/forbid-foreign-prop-types.md | 30 ++++ index.js | 1 + lib/rules/forbid-foreign-prop-types.js | 63 ++++++++ tests/lib/rules/forbid-foreign-prop-types.js | 142 +++++++++++++++++++ 5 files changed, 237 insertions(+) create mode 100644 docs/rules/forbid-foreign-prop-types.md create mode 100644 lib/rules/forbid-foreign-prop-types.js create mode 100644 tests/lib/rules/forbid-foreign-prop-types.js diff --git a/README.md b/README.md index f5bf352603..c2d5f14409 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/docs/rules/forbid-foreign-prop-types.md b/docs/rules/forbid-foreign-prop-types.md new file mode 100644 index 0000000000..1973c92af3 --- /dev/null +++ b/docs/rules/forbid-foreign-prop-types.md @@ -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. diff --git a/index.js b/index.js index df651dfbd1..ca63311435 100644 --- a/index.js +++ b/index.js @@ -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'), diff --git a/lib/rules/forbid-foreign-prop-types.js b/lib/rules/forbid-foreign-prop-types.js new file mode 100644 index 0000000000..f1b7737674 --- /dev/null +++ b/lib/rules/forbid-foreign-prop-types.js @@ -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; + } + + 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' + }); + } + } + }; + } +}; diff --git a/tests/lib/rules/forbid-foreign-prop-types.js b/tests/lib/rules/forbid-foreign-prop-types.js new file mode 100644 index 0000000000..a060f6e170 --- /dev/null +++ b/tests/lib/rules/forbid-foreign-prop-types.js @@ -0,0 +1,142 @@ +/** + * @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'); + +// ----------------------------------------------------------------------------- +// 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 ;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: ERROR_MESSAGE, + type: 'Identifier' + }] + }, + { + code: [ + 'var Foo = React.createClass({', + ' propTypes: Bar["propTypes"],', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: ERROR_MESSAGE, + type: 'Literal' + }] + }, + { + code: [ + 'var { propTypes } = SomeComponent', + 'var Foo = React.createClass({', + ' propTypes,', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: ERROR_MESSAGE, + type: 'Property' + }] + }, + { + code: [ + 'var { propTypes: farts, ...foo } = SomeComponent', + 'var Foo = React.createClass({', + ' propTypes,', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: ERROR_MESSAGE, + type: 'Property' + }] + }, + { + code: [ + 'var { propTypes: typesOfProps } = SomeComponent', + 'var Foo = React.createClass({', + ' propTypes: typesOfProps,', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: ERROR_MESSAGE, + type: 'Property' + }] + }] +});