Skip to content

Commit

Permalink
Merge pull request #1690 from mindspacepdx/am-should-component-update
Browse files Browse the repository at this point in the history
[Fix] Rule prop-types includes nextProps checking in shouldComponentUpdate
  • Loading branch information
ljharb committed Feb 19, 2018
2 parents e26b7e2 + 8ca344a commit c996740
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
28 changes: 26 additions & 2 deletions lib/rules/prop-types.js
Expand Up @@ -121,6 +121,24 @@ module.exports = {
return false;
}

/**
* Check if we are in a class constructor
* @return {boolean} true if we are in a class constructor, false if not
*/
function inShouldComponentUpdate() {
let scope = context.getScope();
while (scope) {
if (
scope.block && scope.block.parent &&
scope.block.parent.key && scope.block.parent.key.name === 'shouldComponentUpdate'
) {
return true;
}
scope = scope.upper;
}
return false;
}

/**
* Checks if a prop is being assigned a value props.bar = 'bar'
* @param {ASTNode} node The AST node being checked.
Expand All @@ -146,7 +164,7 @@ module.exports = {
node.object.type === 'ThisExpression' && node.property.name === 'props'
);
const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node);
const isNextPropsUsage = node.object.name === 'nextProps' && inComponentWillReceiveProps();
const isNextPropsUsage = node.object.name === 'nextProps' && (inComponentWillReceiveProps() || inShouldComponentUpdate());
return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage;
}

Expand Down Expand Up @@ -549,7 +567,9 @@ module.exports = {
const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component();
const isNotInConstructor = !inConstructor();
const isNotInComponentWillReceiveProps = !inComponentWillReceiveProps();
if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps) {
const isNotInShouldComponentUpdate = !inShouldComponentUpdate();
if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps
&& isNotInShouldComponentUpdate) {
return void 0;
}
if (!isDirectProp) {
Expand Down Expand Up @@ -1043,6 +1063,10 @@ module.exports = {
markPropTypesAsUsed(node);
}

if (node.key.name === 'shouldComponentUpdate' && destructuring) {
markPropTypesAsUsed(node);
}

if (!node.static || node.kind !== 'get' || !propsUtil.isPropTypesDeclaration(node)) {
return;
}
Expand Down
42 changes: 42 additions & 0 deletions tests/lib/rules/prop-types.js
Expand Up @@ -3148,6 +3148,48 @@ ruleTester.run('prop-types', rule, {
errors: [
{message: '\'foo\' is missing in props validation'}
]
}, {
code: [
'class Hello extends Component {',
' static propTypes = forbidExtraProps({',
' bar: PropTypes.func',
' })',
' shouldComponentUpdate(nextProps) {',
' if (nextProps.foo) {',
' return;',
' }',
' }',
' render() {',
' return <div bar={this.props.bar} />;',
' }',
'}'
].join('\n'),
parser: 'babel-eslint',
settings: Object.assign({}, settings, {
propWrapperFunctions: ['forbidExtraProps']
}),
errors: [
{message: '\'foo\' is missing in props validation'}
]
}, {
code: [
'class Hello extends Component {',
' shouldComponentUpdate({foo}) {',
' if (foo) {',
' return;',
' }',
' }',
' render() {',
' return <div bar={this.props.bar} />;',
' }',
'}',
'Hello.propTypes = {',
' bar: PropTypes.func',
' }'
].join('\n'),
errors: [
{message: '\'foo\' is missing in props validation'}
]
}, {
code: [
'class Hello extends React.Component {',
Expand Down

0 comments on commit c996740

Please sign in to comment.