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

[Fix] destructuring-assignment, no-multi-comp, no-unstable-nested-components, component detection: improve component detection #3001

Merged
merged 1 commit into from Jun 7, 2021
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
4 changes: 3 additions & 1 deletion CHANGELOG.md
Expand Up @@ -6,11 +6,13 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
## Unreleased

### Fixed
* component detection: use `estraverse` to improve component detection ([#2992][] @Wesitos)
* component detection: use `estraverse` to improve component detection ([#2992][] @Wesitos)
* [`destructuring-assignment`], [`no-multi-comp`], [`no-unstable-nested-components`], component detection: improve component detection ([#3001][] @vedadeepta)

### Changed
* [Docs] [`jsx-no-bind`]: updates discussion of refs ([#2998][] @dimitropoulos)

[#3001]: https://github.com/yannickcr/eslint-plugin-react/pull/3001
[#2998]: https://github.com/yannickcr/eslint-plugin-react/pull/2998
[#2992]: https://github.com/yannickcr/eslint-plugin-react/pull/2992

Expand Down
18 changes: 18 additions & 0 deletions lib/util/Components.js
Expand Up @@ -651,6 +651,8 @@ function componentRule(rule, context) {
return undefined;
}
if (utils.isInAllowedPositionForComponent(node) && utils.isReturningJSXOrNull(node)) {
if (utils.isParentComponentNotStatelessComponent(node)) return undefined;

const isMethod = node.parent.type === 'Property' && node.parent.method;

if (isMethod && !isFirstLetterCapitalized(node.parent.key.name)) {
Expand Down Expand Up @@ -798,6 +800,18 @@ function componentRule(rule, context) {

// Return the component
return components.add(componentNode, 1);
},

isParentComponentNotStatelessComponent(node) {
return (
node.parent
&& node.parent.key
&& node.parent.key.type === 'Identifier'
// custom component functions must start with a capital letter (returns false otherwise)
&& node.parent.key.name.charAt(0) === node.parent.key.name.charAt(0).toLowerCase()
// react render function cannot have params
&& !!(node.params || []).length
);
}
};

Expand Down Expand Up @@ -846,6 +860,7 @@ function componentRule(rule, context) {
components.add(node, 0);
return;
}

const component = utils.getParentComponent();
if (
!component
Expand All @@ -863,6 +878,7 @@ function componentRule(rule, context) {
components.add(node, 0);
return;
}

node = utils.getParentComponent();
if (!node) {
return;
Expand All @@ -875,7 +891,9 @@ function componentRule(rule, context) {
components.add(node, 0);
return;
}

const component = utils.getParentComponent();

if (
!component
|| (component.parent && component.parent.type === 'JSXExpressionContainer')
Expand Down
59 changes: 59 additions & 0 deletions tests/lib/rules/destructuring-assignment.js
Expand Up @@ -196,6 +196,43 @@ ruleTester.run('destructuring-assignment', rule, {
},
};
`
}, {
code: `
const columns = [
{
render: (val) => {
if (val.url) {
return (
<a href={val.url}>
{val.test}
</a>
);
}
return null;
},
},
];
`
}, {
code: `
const columns = [
{
render: val => <span>{val}</span>,
},
{
someRenderFunc: function(val) {
if (val.url) {
return (
<a href={val.url}>
{val.test}
</a>
);
}
return null;
},
},
];
`
}],

invalid: [{
Expand Down Expand Up @@ -353,5 +390,27 @@ ruleTester.run('destructuring-assignment', rule, {
messageId: 'noDestructAssignment',
data: {type: 'state'}
}]
}, {
code: `
const columns = [
{
CustomComponentName: function(props) {
if (props.url) {
return (
<a href={props.url}>
{props.test}
</a>
);
}
return null;
},
},
];
`,
parser: parsers.BABEL_ESLINT,
errors: [{
messageId: 'useDestructAssignment',
data: {type: 'props'}
}]
}]
});
4 changes: 2 additions & 2 deletions tests/lib/rules/no-multi-comp.js
Expand Up @@ -316,11 +316,11 @@ ruleTester.run('no-multi-comp', rule, {
}, {
code: [
'export default {',
' renderHello(props) {',
' RenderHello(props) {',
' let {name} = props;',
' return <div>{name}</div>;',
' },',
' renderHello2(props) {',
' RenderHello2(props) {',
' let {name} = props;',
' return <div>{name}</div>;',
' }',
Expand Down
29 changes: 14 additions & 15 deletions tests/lib/rules/no-unstable-nested-components.js
Expand Up @@ -490,6 +490,20 @@ ruleTester.run('no-unstable-nested-components', rule, {
},
});
`
},
{
code: `
function ParentComponent() {
const rows = [
{
name: 'A',
notPrefixedWithRender: (props) => <Row {...props} />
},
];

return <Table rows={rows} />;
}
`
}
/* TODO These minor cases are currently falsely marked due to component detection
{
Expand Down Expand Up @@ -1010,21 +1024,6 @@ ruleTester.run('no-unstable-nested-components', rule, {
`,
errors: [{message: ERROR_MESSAGE_COMPONENT_AS_PROPS}]
},
{
code: `
function ParentComponent() {
const rows = [
{
name: 'A',
notPrefixedWithRender: (props) => <Row {...props} />
},
];

return <Table rows={rows} />;
}
`,
errors: [{message: ERROR_MESSAGE}]
},
{
code: `
class ParentComponent extends React.Component {
Expand Down