-
-
Notifications
You must be signed in to change notification settings - Fork 32
refactor: update component and hook collectors to return arrays inste… #1334
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ad of maps - Changed `getAllComponents` in `useComponentCollector` and `useComponentCollectorLegacy` to return arrays of components instead of maps. - Updated documentation to reflect the new return types. - Adjusted all usages of `getAllComponents` and `getAllHooks` across the codebase to accommodate the new array structure. - Cleaned up related rules in the ESLint plugins to ensure compatibility with the updated collectors.
162fc70 to
415293b
Compare
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.
Pull request overview
This PR refactors the component and hook collectors to return arrays instead of Maps, simplifying the API and consumer code. While most changes are straightforward conversions from map iteration to array iteration, there are critical issues that need to be addressed before merging.
Key Changes:
- Modified
getAllComponents()andgetAllHooks()in core collectors to return arrays instead of Maps - Updated all rule implementations to iterate over arrays instead of using
.values()on Maps - Fixed broken README links that referenced a branch name instead of
main
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugins/eslint-plugin/README.md | Fixed broken links from branch references to main |
| packages/plugins/eslint-plugin-react-x/src/rules/prefer-read-only-props.ts | Updated to iterate directly over array from getAllComponents() |
| packages/plugins/eslint-plugin-react-x/src/rules/prefer-destructuring-assignment.ts | Refactored to create Set from array for efficient lookups |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unused-props.ts | Simplified by removing map variable and iterating directly |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unstable-default-props.ts | Updated to iterate directly over array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unstable-context-value.ts | Removed .values() call and iterate directly |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unsafe-component-will-update.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unsafe-component-will-receive-props.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unsafe-component-will-mount.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-prefix.ts | Updated to iterate directly over hooks array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-redundant-should-component-update.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-nested-lazy-component-declarations.ts | Removed spread operator wrapping as collectors now return arrays |
| packages/plugins/eslint-plugin-react-x/src/rules/no-nested-component-definitions.ts | Renamed collector variables for clarity and removed array spreading |
| packages/plugins/eslint-plugin-react-x/src/rules/no-missing-component-display-name.ts | Updated iteration and refactored id computation, but contains debugging code |
| packages/plugins/eslint-plugin-react-x/src/rules/no-missing-component-display-name.spec.ts | Critical: All test cases commented out |
| packages/plugins/eslint-plugin-react-x/src/rules/no-component-will-update.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-component-will-receive-props.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-component-will-mount.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-x/src/rules/no-class-component.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-naming-convention/src/rules/component-name.ts | Renamed collector variables and updated to use array iteration |
| packages/plugins/eslint-plugin-react-debug/src/rules/hook.ts | Simplified iteration over hooks array |
| packages/plugins/eslint-plugin-react-debug/src/rules/function-component.ts | Simplified iteration over components array |
| packages/plugins/eslint-plugin-react-debug/src/rules/class-component.ts | Simplified iteration over components array |
| packages/core/src/hook/hook-collector.ts | Changed return type from Map to array by spreading values |
| packages/core/src/component/component-collector.ts | Changed return type from Map to array and removed JSX validation logic from onFunctionExit |
| packages/core/src/component/component-collector-legacy.ts | Changed return type from Map to array by spreading values |
| packages/core/docs/@eslint-react/namespaces/useHookCollector/type-aliases/ReturnType.md | Updated documentation to reflect array return type |
| packages/core/docs/@eslint-react/namespaces/useComponentCollectorLegacy/type-aliases/ReturnType.md | Updated documentation to reflect array return type |
| packages/core/docs/@eslint-react/namespaces/useComponentCollector/type-aliases/ReturnType.md | Updated documentation to reflect array return type |
Comments suppressed due to low confidence (2)
packages/plugins/eslint-plugin-react-x/src/rules/no-missing-component-display-name.spec.ts:65
- All invalid test cases are commented out. These tests should be uncommented and verified to pass with the refactored code. The rule needs test coverage for its invalid cases.
{
code: tsx`
const App = React.memo(() => <div>foo</div>)
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
{
code: tsx`
const App = React.memo(function () {
return <div>foo</div>
})
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
{
code: tsx`
const App = React.forwardRef(() => <div>foo</div>)
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
{
code: tsx`
const MemoComponent = React.memo(() => <div></div>)
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
{
code: tsx`
const ForwardRefComponent = React.forwardRef(() => <div></div>)
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
{
code: tsx`
const MemoForwardRefComponent = React.memo(forwardRef(() => <div></div>))
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
{
code: tsx`
const MemoForwardRefComponent = React.memo(React.forwardRef(() => <div></div>))
`,
errors: [{
messageId: "noMissingComponentDisplayName",
}],
},
packages/plugins/eslint-plugin-react-x/src/rules/no-missing-component-display-name.spec.ts:176
- Most valid test cases are commented out. These tests should be uncommented and verified to pass with the refactored code to ensure proper test coverage.
...allFunctions,
"const App = () => <div>foo</div>",
tsx`
function App() {
return <div>foo</div>
}
`,
tsx`
function App() {
return <div>foo</div>
}
App.displayName = "TestDisplayName";
`,
tsx`
import { memo } from 'react'
const App = memo(function App() {
return <div>foo</div>
})
`,
tsx`
const App = forwardRef(function App() {
return <div>foo</div>
})
`,
tsx`
const App = React.memo(function () {
return <div>foo</div>
})
App.displayName = "TestDisplayName";
`,
tsx`
const App = React.memo(function () {
return <div>foo</div>
})
App.displayName = \`\${"TestDisplayName"}\`;
`,
tsx`
const App = React.memo(function () {
return <div>foo</div>
})
const displayName = "TestDisplayName";
App.displayName = displayName;
`,
tsx`
const someThing = {
displayName: "someThing",
}
const Component = React.forwardRef(() => <div/>)
Component.displayName = someThing.displayName
`,
tsx`
function getDisplayName() { return "someThing" }
const Component = React.forwardRef(() => <div/>)
Component.displayName = getDisplayName()
`,
tsx`
function getDisplayName() { return "someThing" }
const Component = React.forwardRef(() => <div/>)
Component.displayName = (true, 1 + 1, getDisplayName)()
`,
// https://github.com/Rel1cx/eslint-react/issues/177
tsx`
import { forwardRef, ReactNode } from 'react';
interface Props {
children?: ReactNode;
}
export type Ref = HTMLButtonElement;
const FancyButton = forwardRef<Ref, Props>((props, ref) => {
// It's works without this
if (!props.children) return null;
return (
<button ref={ref} className="MyClassName" type="button">
{props.children}
hell
</button>
);
});
FancyButton.displayName = 'FancyButton';
`,
tsx`
import { forwardRef, ReactNode } from 'react';
interface Props {
children?: ReactNode;
}
export type Ref = HTMLButtonElement;
const FancyButton = forwardRef<Ref, Props>((props, ref) => {
// It's works without this
if (!props.children) return <div>foo</div>;
return (
<button ref={ref} className="MyClassName" type="button">
{props.children}
hell
</button>
);
});
FancyButton.displayName = 'FancyButton';
`,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/plugins/eslint-plugin-react-x/src/rules/no-missing-component-display-name.ts
Outdated
Show resolved
Hide resolved
packages/plugins/eslint-plugin-react-x/src/rules/no-missing-component-display-name.ts
Show resolved
Hide resolved
…mponent-display-name.ts Co-authored-by: Copilot <[email protected]> Signed-off-by: REL1CX <[email protected]>
…ad of maps
getAllComponentsinuseComponentCollectoranduseComponentCollectorLegacyto return arrays of components instead of maps.getAllComponentsandgetAllHooksacross the codebase to accommodate the new array structure.What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Checklist
fix: remove a typo, closes #___, #___)Other information