Skip to content

Commit

Permalink
[Fix] prop-types/function-component-definition: Add check for fir…
Browse files Browse the repository at this point in the history
…st letter capitalization in functional component detection

Fixes #2554. Fixes #2495. Fixes #2607. Fixes #2352.
  • Loading branch information
jzabala authored and ljharb committed Jul 6, 2020
1 parent 5bab611 commit ee4bad3
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 5 deletions.
24 changes: 20 additions & 4 deletions lib/util/Components.js
Expand Up @@ -70,6 +70,14 @@ function isReturnsLogicalJSX(node, property, strict) {
: (returnsLogicalJSXLeft || returnsLogicalJSXRight);
}

function isFirstLetterCapitalized(word) {
if (!word) {
return false;
}
const firstLetter = word.charAt(0);
return firstLetter.toUpperCase() === firstLetter;
}

const Lists = new WeakMap();

/**
Expand Down Expand Up @@ -624,13 +632,21 @@ function componentRule(rule, context) {
* @returns {ASTNode | undefined}
*/
getStatelessComponent(node) {
if (node.type === 'FunctionDeclaration') {
if (utils.isReturningJSXOrNull(node)) {
return node;
}
if (
node.type === 'FunctionDeclaration'
&& isFirstLetterCapitalized(node.id.name)
&& utils.isReturningJSXOrNull(node)
) {
return node;
}

if (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') {
if (node.parent.type === 'VariableDeclarator' && utils.isReturningJSXOrNull(node)) {
if (isFirstLetterCapitalized(node.parent.id.name)) {
return node;
}
return undefined;
}
if (utils.isInAllowedPositionForComponent(node) && utils.isReturningJSXOrNull(node)) {
return node;
}
Expand Down
26 changes: 26 additions & 0 deletions tests/lib/rules/function-component-definition.js
Expand Up @@ -68,6 +68,32 @@ ruleTester.run('function-component-definition', rule, {
}, {
code: 'var Foo = React.memo(function Foo() { return <p/> })',
options: [{namedComponents: 'function-declaration'}]
}, {
// shouldn't trigger this rule since functions stating with a lowercase
// letter are not considered components
code: `
const selectAvatarByUserId = (state, id) => {
const user = selectUserById(state, id)
return null
}
`,
options: [{namedComponents: 'function-declaration'}]
}, {
// shouldn't trigger this rule since functions stating with a lowercase
// letter are not considered components
code: `
function ensureValidSourceType(sourceType: string) {
switch (sourceType) {
case 'ALBUM':
case 'PLAYLIST':
return sourceType;
default:
return null;
}
}
`,
options: [{namedComponents: 'arrow-function'}],
parser: parsers.TYPESCRIPT_ESLINT
}, {
code: 'function Hello(props: Test) { return <p/> }',
options: [{namedComponents: 'function-declaration'}],
Expand Down
22 changes: 21 additions & 1 deletion tests/lib/rules/prop-types.js
Expand Up @@ -2504,7 +2504,27 @@ ruleTester.run('prop-types', rule, {
}
`,
parser: parsers.TYPESCRIPT_ESLINT
}
},
// shouldn't trigger this rule since functions stating with a lowercase
// letter are not considered components
`
function noAComponent(props) {
return <div>{props.text}</div>
}
`,
// shouldn't trigger this rule for 'render' since functions stating with a lowercase
// letter are not considered components
`
const MyComponent = (props) => {
const render = () => {
return <test>{props.hello}</test>;
}
return render();
};
MyComponent.propTypes = {
hello: PropTypes.string.isRequired,
};
`
],

invalid: [
Expand Down

0 comments on commit ee4bad3

Please sign in to comment.