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] display-name: fix misinterpreted function components #3065

Conversation

@danielfinke
Copy link
Contributor

@danielfinke danielfinke commented Sep 1, 2021

  • when determining if an ArrowFunctionExpression returns JSX, do not check arguments to a returned CallExpression

Arrow function expressions that return the result of a function call taking JSX arguments are incorrectly deemed to be returning JSX. This leads to some misinterpretations of arrow functions as function components, which are then flagged as "missing display name".

Examples:

ArrowFunctionExpression as Property value

const obj = {
  prop: () => console.log(<a>something</a>)
};
const obj2 = {
  prop: () => { return console.log(<a>something</a>); }
};

ArrowFunctionExpression as RHS of AssignmentExpression

let obj3;
obj3 = () => console.log(<a>something</a>);

I think any other examples of ArrowFunctionExpressions that pass utils.isInAllowedPositionForComponent at

if (!(utils.isInAllowedPositionForComponent(node) && utils.isReturningJSXOrNull(node))) {
return undefined;
}
will hit this too (excl. a VariableDeclarator as it's handled on L635 ahead of time).

@danielfinke danielfinke force-pushed the fix/display-name-misinterpreted-component branch from 228b438 to 0ed3bdc Sep 1, 2021
tests/util/jsx.js Show resolved Hide resolved
tests/util/jsx.js Show resolved Hide resolved
@codecov-commenter
Copy link

@codecov-commenter codecov-commenter commented Sep 1, 2021

Codecov Report

Merging #3065 (1a5a970) into master (810806e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3065   +/-   ##
=======================================
  Coverage   97.43%   97.43%           
=======================================
  Files         111      111           
  Lines        7522     7523    +1     
  Branches     2767     2767           
=======================================
+ Hits         7329     7330    +1     
  Misses        193      193           
Impacted Files Coverage Δ
lib/util/jsx.js 92.75% <100.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 810806e...1a5a970. Read the comment docs.

ljharb
ljharb approved these changes Sep 1, 2021
@ljharb
Copy link
Collaborator

@ljharb ljharb commented Sep 1, 2021

@danielfinke unfortunately since you made this PR from a fork you don't own, i can't force push to it due to a github bug.

Please do not close this PR and open a new one; what would be ideal is if you can add me to https://github.com/Unity-Technologies/eslint-plugin-react for long enough that I can get this PR landed.

@danielfinke
Copy link
Contributor Author

@danielfinke danielfinke commented Sep 1, 2021

@ljharb I'll see what I can do - not sure what hoops I have to jump thru for that.

If there's anything simple you need changed to the commit message, etc., I can do that on your behalf (as that may be quicker). Let me know.

CHANGELOG.md Outdated Show resolved Hide resolved
@ljharb
Copy link
Collaborator

@ljharb ljharb commented Sep 1, 2021

You may also be able to check the "allow edits" box in the RHS of the PR, but github's bug means that probably won't work (but is worth a shot)

- when determining if an `ArrowFunctionExpression` returns JSX, do not check arguments to a returned `CallExpression`
@danielfinke danielfinke force-pushed the fix/display-name-misinterpreted-component branch from 0ed3bdc to 1a5a970 Sep 1, 2021
@ljharb ljharb merged commit 1a5a970 into yannickcr:master Sep 1, 2021
122 of 123 checks passed
@ljharb
Copy link
Collaborator

@ljharb ljharb commented Sep 1, 2021

Thanks! It'd be great if in the future you could make PRs from your own personal fork :-)

@danielfinke danielfinke deleted the fix/display-name-misinterpreted-component branch Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants