-
Notifications
You must be signed in to change notification settings - Fork 279
Integrate eslint react-fc-component-definition, react-render-function-definition
#5902
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
base: develop
Are you sure you want to change the base?
Conversation
swansontec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work, but I found two optional cleanups we could do.
| // Handle `null` in union types - not a JSX type itself | ||
| if (node.type === 'TSNullKeyword') { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block does nothing - we're going to return false either way.
| } | ||
| // Handle qualified names like `React.ReactElement` or `React.JSX.Element` | ||
| if (typeName.type === 'TSQualifiedName') { | ||
| return getTypeName(typeName.left) + '.' + typeName.right.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are going to return strings like "React.ReactElement", should we be more explicit about putting full type names in the JSX_RETURN_TYPES array? I see that we use .endsWith to do the check, so we could alternatively just return typeName.right.name and skip the recursion, since we don't actually care what's on the left.
d0645d5 to
805218f
Compare
805218f to
2988c9b
Compare
Enforces that render helper functions (camelCase functions starting with "render") use React.ReactElement return type instead of ReactNode or JSX.Element. - Flags ReactNode (too broad - use ReactElement or explicit union) - Flags JSX.Element (use ReactElement for consistency) - Allows ReactElement, ReactElement | null, and other explicit unions
|
Updated description, added one more rule |
react-fc-component-definitionreact-fc-component-definition, react-render-function-definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| // Handle `React.ReactElement`, `React.JSX.Element`, etc. | ||
| if (node.type === 'TSTypeReference') { | ||
| const typeName = getTypeName(node.typeName) | ||
| return JSX_RETURN_TYPES.some(t => typeName.endsWith(t)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type matching incorrectly flags non-JSX types ending with "Element"
Low Severity
The .endsWith() check combined with 'Element' in JSX_RETURN_TYPES causes false positives. Since getTypeName returns only the rightmost identifier (e.g., 'HTMLElement' for the type HTMLElement), the check 'HTMLElement'.endsWith('Element') evaluates to true. This incorrectly flags PascalCase functions returning HTMLElement, SVGElement, or any custom type ending with Element as needing React.FC<Props>. The second rule correctly uses .includes() for exact matching, but this rule's .endsWith() approach is too broad.
edge/react-fc-component-definition & edge/react-render-function-definition
Two complementary ESLint rules enforcing consistent React component and render function typing conventions.
Rule Details
edge/react-fc-component-definitionTargets PascalCase-named arrow functions that return JSX types. Components should use
React.FC<Props>instead of explicit return types.edge/react-render-function-definitionTargets camelCase functions starting with
render(e.g.,renderItem,renderHeader). Render helpers should useReact.ReactElementinstead ofReactNodeorJSX.Element.Why These Rules?
For Components (
React.FC<Props>):React.FChandles thechildrenprop type automaticallyReact.FC, reducing verbosityFor Render Helpers (
React.ReactElement):ReactNodeis too broad (includes strings, numbers, arrays)JSX.Elementshould beReactElementfor uniformityReactElement | nullExamples
Components (PascalCase)
Incorrect
Correct
Render Helpers (camelCase)
Incorrect
Correct
Configuration
Both rules are enabled as warnings by default in the Edge ESLint configuration:
Related
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: