test: enhance test coverage for eslint-plugin-react-x and eslint-plugin-react-dom rules#1663
Merged
test: enhance test coverage for eslint-plugin-react-x and eslint-plugin-react-dom rules#1663
Conversation
…in-react-dom rules - Add comprehensive tests for no-nested-lazy-component-declarations - Strengthen rule to detect lazy declarations in hooks - Add tests for no-set-state-in-component-did-mount - Add tests for no-set-state-in-component-did-update - Add tests for no-set-state-in-component-will-update - Add tests for no-direct-mutation-state - Add tests for no-flush-sync - Add tests for no-script-url - Add tests for no-string-style-prop - Add tests for no-unsafe-iframe-sandbox - Add additionalHooks config tests for exhaustive-deps
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Expands rule test coverage across eslint-plugin-react-x and eslint-plugin-react-dom, and updates no-nested-lazy-component-declarations to also flag lazy declarations inside hooks.
Changes:
- Added many new invalid/valid test cases for several existing rules (setState lifecycle rules, direct state mutation, DOM/security rules, and
flushSync). - Updated
no-nested-lazy-component-declarationsrule logic to include hook detection via the hook collector. - Added
additionalHooksconfiguration tests forexhaustive-deps.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugins/eslint-plugin-react-x/src/rules/no-set-state-in-component-will-update/no-set-state-in-component-will-update.spec.ts | Adds broader invalid/valid coverage around componentWillUpdate cases. |
| packages/plugins/eslint-plugin-react-x/src/rules/no-set-state-in-component-did-update/no-set-state-in-component-did-update.spec.ts | Adds broader invalid/valid coverage around componentDidUpdate cases. |
| packages/plugins/eslint-plugin-react-x/src/rules/no-set-state-in-component-did-mount/no-set-state-in-component-did-mount.spec.ts | Adds broader invalid/valid coverage around componentDidMount cases. |
| packages/plugins/eslint-plugin-react-x/src/rules/no-nested-lazy-component-declarations/no-nested-lazy-component-declarations.ts | Extends detection to hooks using getHookCollector. |
| packages/plugins/eslint-plugin-react-x/src/rules/no-nested-lazy-component-declarations/no-nested-lazy-component-declarations.spec.ts | Adds comprehensive invalid/valid scenarios for nested lazy() declarations (components/hooks/JSX contexts). |
| packages/plugins/eslint-plugin-react-x/src/rules/no-direct-mutation-state/no-direct-mutation-state.spec.ts | Adds more mutation patterns and clarifies allowed constructor/property initialization scenarios. |
| packages/plugins/eslint-plugin-react-x/src/rules/exhaustive-deps/exhaustive-deps.spec.ts | Adds additionalHooks option coverage for custom-effect-like hooks. |
| packages/plugins/eslint-plugin-react-dom/src/rules/no-unsafe-iframe-sandbox/no-unsafe-iframe-sandbox.spec.ts | Adds many unsafe/safe sandbox combinations, whitespace variants, and value forms. |
| packages/plugins/eslint-plugin-react-dom/src/rules/no-string-style-prop/no-string-style-prop.spec.ts | Adds many host-element cases and more forms of string style values. |
| packages/plugins/eslint-plugin-react-dom/src/rules/no-script-url/no-script-url.spec.ts | Adds protocol variants, whitespace/control char cases, and broader element/attribute coverage. |
| packages/plugins/eslint-plugin-react-dom/src/rules/no-flush-sync/no-flush-sync.spec.ts | Adds more call-shape coverage and introduces a valid aliasing case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -3,6 +3,7 @@ import * as core from "@eslint-react/core"; | |||
| import { type RuleContext, type RuleFeature, defineRuleListener } from "@eslint-react/shared"; | |||
| import type { TSESTree } from "@typescript-eslint/types"; | |||
| import { AST_NODE_TYPES as AST } from "@typescript-eslint/types"; | |||
There was a problem hiding this comment.
AST_NODE_TYPES as AST is now unused after simplifying the parent-checking logic. Consider removing this import to avoid dead code and keep the rule file tidy.
Suggested change
| import { AST_NODE_TYPES as AST } from "@typescript-eslint/types"; |
Comment on lines
+219
to
+223
| // Aliased import is not detected by this rule (limitation of simple text search) | ||
| { | ||
| code: tsx` | ||
| import { flushSync as fs } from 'react-dom'; | ||
|
|
There was a problem hiding this comment.
This comment is misleading: the rule does run on the file (because the import still contains the text flushSync), but it doesn't report because the callee is aliased to fs and the rule only matches callees named flushSync. Please reword to reflect the actual limitation (aliasing), not the fast-path text check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Checklist
fix: remove a typo, closes #___, #___)Other information