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 allowInPropTypes option to forbid-foreign-prop-types (#1647) #1655

Merged
merged 1 commit into from
Jan 29, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 63 additions & 16 deletions lib/rules/forbid-foreign-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,32 @@

const docsUrl = require('../util/docsUrl');

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


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

module.exports = {
meta: {
docs: {
description: 'Forbid using another component\'s propTypes',
category: 'Best Practices',
recommended: false,
url: docsUrl('forbid-foreign-prop-types')
}
},

schema: [
{
type: 'object',
properties: {
allowInPropTypes: {
type: 'boolean'
}
},
additionalProperties: false
}
]
},

create: function(context) {
const config = context.options[0] || {};
const allowInPropTypes = config.allowInPropTypes || false;

// --------------------------------------------------------------------------
// Helpers
// --------------------------------------------------------------------------
Expand All @@ -34,15 +40,56 @@ module.exports = {
return node.parent.type === 'AssignmentExpression' && node.parent.left === node;
}

function findParentAssignmentExpression(node) {
let parent = node.parent;

while (parent && parent.type !== 'Program') {
if (parent.type === 'AssignmentExpression') {
return parent;
}
parent = parent.parent;
}
return null;
}

function isAllowedAssignment(node) {
if (!allowInPropTypes) {
return false;
}

const assignmentExpression = findParentAssignmentExpression(node);

if (
assignmentExpression &&
assignmentExpression.left &&
assignmentExpression.left.property &&
assignmentExpression.left.property.name === 'propTypes'
) {
return true;
}
return false;
}

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)) {
if (
node.property &&
(
!node.computed &&
node.property.type === 'Identifier' &&
node.property.name === 'propTypes' &&
!isLeftSideOfAssignment(node) &&
!isAllowedAssignment(node)
) || (
node.property.type === 'Literal' &&
node.property.value === 'propTypes' &&
!isLeftSideOfAssignment(node) &&
!isAllowedAssignment(node)
)
) {
context.report({
node: node.property,
message: 'Using another component\'s propTypes is forbidden'
message: 'Using propTypes from another component is not safe because they may be removed in production builds'
});
}
},
Expand All @@ -53,7 +100,7 @@ module.exports = {
if (propTypesNode) {
context.report({
node: propTypesNode,
message: 'Using another component\'s propTypes is forbidden'
message: 'Using propTypes from another component is not safe because they may be removed in production builds'
});
}
}
Expand Down
36 changes: 35 additions & 1 deletion tests/lib/rules/forbid-foreign-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require('babel-eslint');
// Tests
// -----------------------------------------------------------------------------

const ERROR_MESSAGE = 'Using another component\'s propTypes is forbidden';
const ERROR_MESSAGE = 'Using propTypes from another component is not safe because they may be removed in production builds';

const ruleTester = new RuleTester({parserOptions});
ruleTester.run('forbid-foreign-prop-types', rule, {
Expand All @@ -50,6 +50,21 @@ ruleTester.run('forbid-foreign-prop-types', rule, {
code: 'Foo["propTypes"] = propTypes'
}, {
code: 'const propTypes = "bar"; Foo[propTypes];'
},
{
code: `
const Message = (props) => (<div>{props.message}</div>);
Message.propTypes = {
message: PropTypes.string
};
const Hello = (props) => (<Message>Hello {props.name}</Message>);
Hello.propTypes = {
name: Message.propTypes.message
};
`,
options: [{
allowInPropTypes: true
}]
}],

invalid: [{
Expand Down Expand Up @@ -139,5 +154,24 @@ ruleTester.run('forbid-foreign-prop-types', rule, {
message: ERROR_MESSAGE,
type: 'Property'
}]
},
{
code: `
const Message = (props) => (<div>{props.message}</div>);
Message.propTypes = {
message: PropTypes.string
};
const Hello = (props) => (<Message>Hello {props.name}</Message>);
Hello.propTypes = {
name: Message.propTypes.message
};
`,
options: [{
allowInPropTypes: false
}],
errors: [{
message: ERROR_MESSAGE,
type: 'Identifier'
}]
}]
});