fix(linter/react): react/display-name no longer flags arrows that only pass JSX as an argument#22755
Conversation
…t returns function that has JSX
Merging this PR will not alter performance
Comparing 
Footnotes
|
camc314
left a comment
camc314
left a comment
There was a problem hiding this comment.
Thanks for the fix. I agree with the underlying issue: react/display-name should not treat an inner arrow as a component just because JSX appears as a call argument.
However, this change narrows the sharedexpression_contains_jsx helper which is also used by the other react rules. It also becomes narrower than upstream eslint-plugin-react: upstream allows the reported someGlobalFunc(<div />) case, but still reports HOFs that return JSX through conditionals, logical expressions, sequences, or JSX-valued identifiers.
For example, upstream still reports:
const renderer = a => listItem => cond ? <div>{listItem}</div> : null;
const renderer = a => listItem => { const x = <div>{listItem}</div>; return x; };This branch no longer reports those cases.
I'll take a look at another approach to fixing this issue
I see where that's coming from. I was also worried about introducing regressions with these changes, but since all of the tests passed, that gave me at least a bit of confidence.
I will also try thinking about another solution. One where we wouldn't have to worry about regressions (at least not obvious ones), and that would also cover these cases you mentioned: const renderer = a => listItem => cond ? <div>{listItem}</div> : null;
const renderer = a => listItem => { const x = <div>{listItem}</div>; return x; };Anyways, thanks for the feedback. If I come up with a different solution than the current one before you, I will push the changes to this PR. |
|
Hey @camc314. I've come up with a different solution, which reverts my previous changes and introduces a new field, I have to admit that I am not really proud of it, since I have a gut feeling that this could've been done in a better, cleaner way. But I might be wrong, please do let me know about it! |
|
I've realized that the changes in this PR are also applicable to #22608. So I've added test cases to also cover them. |
…ct-display-name-false-positive
….com:kapobajza/oxc into fix/linter-react-display-name-false-positive
|
@kapobajza Just wanted to see what the status of this was and if there's anything I could do to help get this across the line since a bug this fixes is impacting us. |
Hey @pbomb. I've come up with a solution that works, the test cases pass, but I am not particularly proud of it. I have a gut feeling that there might be a better way to do it. Right now I am waiting for @camc314 to review it, and if this is fine as it is, we might be able to proceed with it. However, if it's not fine, then we would have to come up with something better. And thank you for offering your help! If you have a better solution in mind, than the one I already implemented, I would be grateful if you could let me know! |
|
@kapobajza @camc314 I (and Claude) created this stacked PR that attempts to address the PR feedback here. I didn't want to push to this branch, so if you like the direction, we can merge it into this one and if not, we can just drop it. |
## Summary - detect React function components from reachable JSX/null returns using a shared CFG utility - avoid function-component-definition false positives for callbacks, lowercase helpers, class methods, and functions that only contain JSX - avoid display-name false positives for higher-order functions that contain but do not return JSX fixes [https://github.com/oxc-project/oxc/issues/22685](https://github.com/oxc-project/oxc/issues/22685) closes [#22755](#22755) follow up to #24471
## Summary - detect React function components from reachable JSX/null returns using a shared CFG utility - avoid function-component-definition false positives for callbacks, lowercase helpers, class methods, and functions that only contain JSX - avoid display-name false positives for higher-order functions that contain but do not return JSX fixes [https://github.com/oxc-project/oxc/issues/22685](https://github.com/oxc-project/oxc/issues/22685) closes [#22755](#22755) follow up to #24471
3 participants
AI usage disclosure
I've used Claude Code to assist me with the solution, and it has been reviewed and tested by me. I've also used it to generate more tests to harden the fix, and avoid introducing regressions.
I've also used it to generate this PR description.
Summary
Closes #22685
Closes #22608
react/display-namereported a false positive for higher-order functions whose inner arrow contains JSX but never returns it:This is a utility function, not a component, so the rule should stay silent.
Cause
expression_contains_jsxdetected JSX anywhere in an arrow function's body (via a full visitor walk), so JSX passed as a call argument was treated the same as JSX returned from the component.Fix
For arrow functions,
expression_contains_jsxnow only inspects the return position — the concise-body expression, or top-levelreturnstatements — instead of the whole body. JSX appearing only as a call argument no longer counts as a component.The full-walk helper (
function_body_contains_jsx) is left untouched and still used by the anonymous-export path and byno-multi-comp/no-unstable-nested-components/prefer-function-component, so their behavior is unchanged.