-
-
Notifications
You must be signed in to change notification settings - Fork 629
feat(linter): implement eslint-plugin-react-refresh/only-export-components rule with full configuration support #12756
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: main
Are you sure you want to change the base?
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
…nents rule Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
CodSpeed Instrumentation Performance ReportMerging #12756 will not alter performanceComparing Summary
|
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.
Review: Implementation needs improvements
Thank you for working on implementing the only-export-components
rule! After reviewing the implementation against the original eslint-plugin-react-refresh, I've identified several areas that need attention:
Missing Features
-
Configuration Options: The implementation is missing all configuration options from the original plugin:
allowConstantExport
: Allow exporting constants alongside componentsallowExportNames
: Allow specific export namescustomHOCs
: Support for custom Higher-Order ComponentscheckJS
: Support for JSX in .js files
-
Limited HOC Support: Currently only supports
memo
andforwardRef
, but missing:connect
from react-reduxwithRouter
from react-router- Other common HOCs
- Custom HOCs via configuration
Implementation Issues
-
Component Name Detection (line 358-360): The regex is too restrictive:
fn is_react_component_name(&self, name: &str) -> bool { name.chars().next().map_or(false, |c| c.is_ascii_uppercase()) && name.chars().all(|c| c.is_ascii_alphanumeric()) }
This doesn't allow underscores in component names (e.g.,
My_Component
). -
Missing Component Types:
- No detection of class components
- No support for function expressions as components
- Missing type-only export handling (TypeScript)
Test Coverage
- Insufficient Test Cases: The current tests don't cover:
- HOC scenarios
- Mixed exports with constants
- React context exports
- Type-only exports
- Configuration options
- Class components
- Nested HOCs
Other Considerations
-
File Detection: The test file detection might miss patterns like:
- Files in
__tests__
directories - Files ending with
.test.js
(without dot before "test")
- Files in
-
Missing Edge Cases:
- No handling of nested HOC calls
- No detection of all-uppercase exports (e.g.,
const CMS = () => {}
)
Recommendation
Consider expanding the implementation to match the original plugin's functionality more closely. The core logic is good, but it needs these additional features to be fully compatible with projects migrating from ESLint to OXC.
Would you like help implementing any of these missing features?
…configuration options and better detection Co-authored-by: Dunqing <29533304+Dunqing@users.noreply.github.com>
I've significantly enhanced the implementation to address all the missing features you identified: ✅ Fixed IssuesComponent Name Detection: Fixed the regex to allow underscores ( Configuration Options: Added all missing options:
Enhanced HOC Support: Now supports Better Component Detection: Added class components, function expressions, anonymous export detection Improved File Detection: Now handles React Context Detection: Proper handling of Type-only Exports: Correctly skips TypeScript type-only exports Comprehensive Testing: Added test coverage for all new features and configuration options The implementation now closely matches the original eslint-plugin-react-refresh functionality while following oxc's patterns. All tests pass and clippy warnings are resolved. Commit: f23ece7 |
This PR implements the
only-export-components
rule from eslint-plugin-react-refresh with comprehensive configuration support to ensure files only export React components for proper Fast Refresh/Hot Module Replacement functionality.What this rule does
The rule validates that files containing React components follow patterns that work correctly with Fast Refresh:
export * from "./foo"
cannot be verified to only export componentsexport default function() {}
break Fast RefreshConfiguration Options
The rule supports all configuration options from the original eslint-plugin-react-refresh:
allowConstantExport
: Allow exporting constants alongside componentsallowExportNames
: Allow specific export names (array of strings)customHOCs
: Support for custom Higher-Order Components (array of strings)checkJS
: Support for JSX in .js files (extends beyond just .jsx/.tsx)Enhanced Features
My_Component
)memo
,forwardRef
,connect
,withRouter
,observer
, plus custom HOCscreateContext()
andReact.createContext()
calls__tests__
directories and various test file patternsExamples
❌ Incorrect - Mixed exports break Fast Refresh:
✅ Correct with allowConstantExport - Constants allowed with configuration:
✅ Correct - Component-only exports work with Fast Refresh:
The implementation follows oxc's patterns and integrates seamlessly with the existing React plugin, providing full compatibility with projects migrating from ESLint to OXC.
Fixes #8698.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.