Skip to content

Commit

Permalink
[Fix] no-unstable-components: improve handling of objects containin…
Browse files Browse the repository at this point in the history
…g render function properties
  • Loading branch information
fizwidget authored and ljharb committed Oct 23, 2021
1 parent 182e95d commit cf47696
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* [`no-unused-prop-types`], `usedPropTypes`: avoid crash with typescript-eslint parser (@ljharb)
* [`display-name`]: unwrap TS `as` expressions ([#3110][] @ljharb)
* [`destructuring-assignment`]: detect refs nested in functions ([#3102] @ljharb)
* [`no-unstable-components`]: improve handling of objects containing render function properties ([#3111] @fizwidget)

[#3111]: https://github.com/yannickcr/eslint-plugin-react/pull/3111
[#3110]: https://github.com/yannickcr/eslint-plugin-react/pull/3110
[#3102]: https://github.com/yannickcr/eslint-plugin-react/issue/3102
[#3092]: https://github.com/yannickcr/eslint-plugin-react/pull/3092
Expand Down
20 changes: 19 additions & 1 deletion lib/rules/no-unstable-nested-components.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,19 @@ function isJSXAttributeOfExpressionContainerMatcher(node) {
);
}

/**
* Matcher used to check whether given node is an object `Property`
* @param {ASTNode} node The AST node
* @returns {Boolean} True if node is a `Property`, false if not
*/
function isPropertyOfObjectExpressionMatcher(node) {
return (
node
&& node.parent
&& node.parent.type === 'Property'
);
}

/**
* Matcher used to check whether given node is a `CallExpression`
* @param {ASTNode} node The AST node
Expand Down Expand Up @@ -358,14 +371,19 @@ module.exports = {
}

/**
* Check whether given node is declared inside a component prop.
* Check whether given node is declared inside a component/object prop.
* ```jsx
* <Component footer={() => <div />} />
* { footer: () => <div /> }
* ```
* @param {ASTNode} node The AST node being checked
* @returns {Boolean} True if node is a component declared inside prop, false if not
*/
function isComponentInProp(node) {
if (isPropertyOfObjectExpressionMatcher(node)) {
return utils.isReturningJSX(node);
}

const jsxAttribute = getClosestMatchingParent(node, context, isJSXAttributeOfExpressionContainerMatcher);

if (!jsxAttribute) {
Expand Down
133 changes: 133 additions & 0 deletions tests/lib/rules/no-unstable-nested-components.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,78 @@ ruleTester.run('no-unstable-nested-components', rule, {
`,
options: [{ allowAsProps: true }],
},
{
code: `
function ParentComponent() {
return (
<SomeComponent>
{
thing.match({
renderLoading: () => <div />,
renderSuccess: () => <div />,
renderFailure: () => <div />,
})
}
</SomeComponent>
)
}
`,
},
{
code: `
function ParentComponent() {
const thingElement = thing.match({
renderLoading: () => <div />,
renderSuccess: () => <div />,
renderFailure: () => <div />,
});
return (
<SomeComponent>
{thingElement}
</SomeComponent>
)
}
`,
},
{
code: `
function ParentComponent() {
return (
<SomeComponent>
{
thing.match({
loading: () => <div />,
success: () => <div />,
failure: () => <div />,
})
}
</SomeComponent>
)
}
`,
options: [{
allowAsProps: true,
}],
},
{
code: `
function ParentComponent() {
const thingElement = thing.match({
loading: () => <div />,
success: () => <div />,
failure: () => <div />,
});
return (
<SomeComponent>
{thingElement}
</SomeComponent>
)
}
`,
options: [{
allowAsProps: true,
}],
},
{
code: `
function ParentComponent() {
Expand Down Expand Up @@ -500,6 +572,9 @@ ruleTester.run('no-unstable-nested-components', rule, {
return <Table rows={rows} />;
}
`,
options: [{
allowAsProps: true,
}],
},
/* TODO These minor cases are currently falsely marked due to component detection
{
Expand Down Expand Up @@ -1042,5 +1117,63 @@ ruleTester.run('no-unstable-nested-components', rule, {
// Only a single error should be shown. This can get easily marked twice.
errors: [{ message: ERROR_MESSAGE }],
},
{
code: `
function ParentComponent() {
return (
<SomeComponent>
{
thing.match({
loading: () => <div />,
success: () => <div />,
failure: () => <div />,
})
}
</SomeComponent>
)
}
`,
errors: [
{ message: ERROR_MESSAGE_COMPONENT_AS_PROPS },
{ message: ERROR_MESSAGE_COMPONENT_AS_PROPS },
{ message: ERROR_MESSAGE_COMPONENT_AS_PROPS },
],
},
{
code: `
function ParentComponent() {
const thingElement = thing.match({
loading: () => <div />,
success: () => <div />,
failure: () => <div />,
});
return (
<SomeComponent>
{thingElement}
</SomeComponent>
)
}
`,
errors: [
{ message: ERROR_MESSAGE_COMPONENT_AS_PROPS },
{ message: ERROR_MESSAGE_COMPONENT_AS_PROPS },
{ 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_COMPONENT_AS_PROPS }],
},
]),
});

0 comments on commit cf47696

Please sign in to comment.