Skip to content
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* [`no-unstable-components`]: improve handling of objects containing render function properties ([#3111] @fizwidget)
* [`prop-types`], `propTypes`: add forwardRef<>, ForwardRefRenderFunction<> prop-types ([#3112] @vedadeepta)
* [`no-typos`]: prevent a crash when using private methods (@ljharb)
* [`destructuring-assignment`], component detection: improve component detection ([#3122] @vedadeepta)

### Changed
* [Tests] test on the new babel eslint parser ([#3113] @ljharb)

[#3122]: https://github.com/yannickcr/eslint-plugin-react/pull/3122
[#3113]: https://github.com/yannickcr/eslint-plugin-react/pull/3113
[#3112]: https://github.com/yannickcr/eslint-plugin-react/pull/3112
[#3111]: https://github.com/yannickcr/eslint-plugin-react/pull/3111
Expand Down
15 changes: 14 additions & 1 deletion lib/util/Components.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,10 @@ function componentRule(rule, context) {
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, context, strict);
},

isReturningOnlyNull(ASTNode) {
return jsxUtil.isReturningOnlyNull(this.isCreateElement.bind(this), ASTNode, context);
},

getPragmaComponentWrapper(node) {
let isPragmaComponentWrapper;
let currentNode = node;
Expand Down Expand Up @@ -556,6 +560,16 @@ function componentRule(rule, context) {
return undefined;
}

// case: function any() { return (props) { return not-jsx-and-not-null } }
if (node.parent.type === 'ReturnStatement' && !utils.isReturningJSX(node) && !utils.isReturningOnlyNull(node)) {
return undefined;
}

// for case abc = { [someobject.somekey]: props => { ... return not-jsx } }
if (node.parent && node.parent.key && node.parent.key.type === 'MemberExpression' && !utils.isReturningJSX(node) && !utils.isReturningOnlyNull(node)) {
return undefined;
}

// Case like `React.memo(() => <></>)` or `React.forwardRef(...)`
const pragmaComponentWrapper = utils.getPragmaComponentWrapper(node);
if (pragmaComponentWrapper) {
Expand Down Expand Up @@ -805,7 +819,6 @@ function componentRule(rule, context) {
}

const component = utils.getParentComponent();

if (
!component
|| (component.parent && component.parent.type === 'JSXExpressionContainer')
Expand Down
48 changes: 48 additions & 0 deletions lib/util/jsx.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,59 @@ function isReturningJSX(isCreateElement, ASTnode, context, strict, ignoreNull) {
return found;
}

/**
* Check if the node is returning only null values
*
* @param {Function} isCreateElement Function to determine if a CallExpresion is
* a createElement one
* @param {ASTNode} ASTnode The AST node being checked
* @param {Context} context The context of `ASTNode`.
* @returns {Boolean} True if the node is returning only null values
*/
function isReturningOnlyNull(isCreateElement, ASTnode, context) {
let found = false;
let foundSomethingElse = false;
astUtil.traverseReturns(ASTnode, context, (node) => {
// Traverse return statement
astUtil.traverse(node, {
enter(childNode) {
const setFound = () => {
found = true;
this.skip();
};
const setFoundSomethingElse = () => {
foundSomethingElse = true;
this.skip();
};
switch (childNode.type) {
case 'ReturnStatement':
break;
case 'ConditionalExpression':
if (childNode.consequent.value === null && childNode.alternate.value === null) {
setFound();
}
break;
case 'Literal':
if (childNode.value === null) {
setFound();
}
break;
default:
setFoundSomethingElse();
}
},
});
});

return found && !foundSomethingElse;
}

module.exports = {
isDOMComponent,
isFragment,
isJSX,
isJSXAttributeKey,
isWhiteSpaces,
isReturningJSX,
isReturningOnlyNull,
};
108 changes: 108 additions & 0 deletions tests/lib/rules/destructuring-assignment.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,56 @@ const parserOptions = {
const ruleTester = new RuleTester({ parserOptions });
ruleTester.run('destructuring-assignment', rule, {
valid: parsers.all([
{
code: `
export const revisionStates2 = {
[A.b]: props => {
return props.editor !== null
? 'xyz'
: 'abc'
},
};
`,
},
{
code: `
export function hof(namespace) {
const initialState = {
bounds: null,
search: false,
};
return (props) => {
const {x, y} = props
if (y) {
return <span>{y}</span>;
}
return <span>{x}</span>
};
}
`,
},
{
code: `
export function hof(namespace) {
const initialState = {
bounds: null,
search: false,
};

return (state = initialState, action) => {
if (action.type === 'ABC') {
return {...state, bounds: stuff ? action.x : null};
}

if (action.namespace !== namespace) {
return state;
}

return null
};
}
`,
},
{
code: `
const MyComponent = ({ id, className }) => (
Expand Down Expand Up @@ -615,5 +665,63 @@ ruleTester.run('destructuring-assignment', rule, {
},
],
},
{
code: `
export const revisionStates2 = {
[A.b]: props => {
return props.editor !== null
? <span>{props.editor}</span>
: null
},
};
`,
parser: parsers.BABEL_ESLINT,
errors: [
{
messageId: 'useDestructAssignment',
data: { type: 'props' },
line: 4,
},
{
messageId: 'useDestructAssignment',
data: { type: 'props' },
line: 5,
},
],
},
{
code: `
export function hof(namespace) {
const initialState = {
bounds: null,
search: false,
};
return (props) => {
if (props.y) {
return <span>{props.y}</span>;
}
return <span>{props.x}</span>
};
}
`,
parser: parsers.BABEL_ESLINT,
errors: [
{
messageId: 'useDestructAssignment',
data: { type: 'props' },
line: 8,
},
{
messageId: 'useDestructAssignment',
data: { type: 'props' },
line: 9,
},
{
messageId: 'useDestructAssignment',
data: { type: 'props' },
line: 11,
},
],
},
]),
});