feat(linter): Implement react/no-react-children rule.#20104
feat(linter): Implement react/no-react-children rule.#20104graphite-app[bot] merged 1 commit intomainfrom
react/no-react-children rule.#20104Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new eslint-plugin-react rule (react/no-react-children) to warn on usage of the legacy React.Children API, aligning oxlint’s React guidance with React’s current recommendations.
Changes:
- Implement
react/no-react-childrenrule logic, diagnostics, and unit tests with snapshot coverage. - Register the rule in the react rules module and in generated rule dispatch/runner code.
- Add a snapshot file capturing expected diagnostics output.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_linter/src/rules/react/no_react_children.rs | New rule implementation, documentation, and tests for detecting React.Children.* usage patterns. |
| crates/oxc_linter/src/rules.rs | Exposes the new rule module under the React rules namespace. |
| crates/oxc_linter/src/generated/rules_enum.rs | Adds the rule to the generated rule enum, IDs, and dispatch helpers. |
| crates/oxc_linter/src/generated/rule_runner_impls.rs | Registers AST node filtering/runner implementation for the new rule. |
| crates/oxc_linter/src/snapshots/react_no_react_children.snap | Snapshot of diagnostics emitted by the new rule’s tests. |
Merging this PR will not alter performance
Comparing Footnotes
|
Merge activity
|
AI Disclosure: I used Claude Code for the implementation of the rule's `run` and `is_imported_from_react` functions. All other code (diagnostics, tests, rule docs, etc.) were written by me. Fixes #20015. Part of #1022, I guess? This rule is based on the following rules from `@eslint-react/eslint-plugin`: - [`@eslint-react/no-children-count`](https://www.eslint-react.xyz/docs/rules/no-children-count) - [`@eslint-react/no-children-for-each`](https://www.eslint-react.xyz/docs/rules/no-children-for-each) - [`@eslint-react/no-children-map`](https://www.eslint-react.xyz/docs/rules/no-children-map) - [`@eslint-react/no-children-only`](https://www.eslint-react.xyz/docs/rules/no-children-only) - [`@eslint-react/no-children-to-array`](https://www.eslint-react.xyz/docs/rules/no-children-to-array) The goal of the rule is to prevent all usage of [`React.Children`](https://react.dev/reference/react/Children), which is discouraged by the React docs. Examples of rule violations: ```jsx import { Children } from 'react'; Children.toArray(children); Children.map(children, child => <div>{child}</div>); Children.only(children); Children.count(children); Children.forEach(children, (child, index) => {}); // --- import React from 'react'; React.Children.toArray(children); ``` A few things to note, in terms of what this rule intentionally does not catch right now: 1. This rule will not catch CommonJS imports like `const { Children } = require('react');`. It didn't really seem worth the trouble. 2. This does not catch alias imports like `import { Children as ReactChildren } from 'react';`. I considered adding that case, but it would make the rule more complex for an edge case, and I wasn't sure that was really worthwhile. The original rule(s) from `@eslint-react/eslint-plugin` also didn't handle this. 3. There are a few other ways to get around this, like destructuring from `React` after importing it: ```jsx import React from 'react'; const { Children } = React; ``` I don't know that that is worth protecting against? We can always add support for complex cases like that in subsequent PRs if desired. I tried to keep it as simple as possible, which means there will be some holes like the above, but it will be much easier to review and validate as a result, and the above patterns are generally uncommon anyway. ----- ecosystem-ci run, no new failures (not that any are even really possible, given this isn't a default rule / doesn't have a fixer): https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/22804771151 For validation purposes, it does add 7 new errors to the bsky results, before: ``` Found 0 warnings and 76795 errors. Finished in 1.5s on 1555 files with 629 rules using 4 threads. ``` after: ``` Found 0 warnings and 76802 errors. Finished in 1.4s on 1555 files with 630 rules using 4 threads. ``` Running the rule on the bsky codebase, all 7 results are correctly-detected violations: <details> <summary>oxlint results with this rule</summary> ``` × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/view/com/pager/Pager.web.tsx:90:8] 89 │ })} 90 │ {Children.map(children, (child, i) => ( · ──────────── 91 │ <View ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/components/FocusScope/index.tsx:52:12] 51 │ const decoratedChildren = useMemo(() => { 52 │ return Children.toArray(children).map((node, i) => { · ──────────────── 53 │ if (i === 0 && isValidElement(node)) { ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/components/forms/InputGroup.tsx:11:20] 10 │ const t = useTheme() 11 │ const children = React.Children.toArray(props.children) · ────────────────────── 12 │ const total = children.length ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/components/Tooltip/index.web.tsx:113:13] 112 │ export function TextBubble({children}: {children: React.ReactNode}) { 113 │ const c = Children.toArray(children) · ──────────────── 114 │ return ( ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/components/Tooltip/index.tsx:455:13] 454 │ export function TextBubble({children}: {children: React.ReactNode}) { 455 │ const c = Children.toArray(children) · ──────────────── 456 │ return ( ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/alf/typography.tsx:71:3] 70 │ let hasEmoji = false 71 │ Children.forEach(children, child => { · ──────────────── 72 │ if (typeof child === 'string' && createEmojiRegex().test(child)) { ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives × eslint-plugin-react(no-react-children): `React.Children` should not be used. ╭─[src/alf/typography.tsx:87:10] 86 │ } 87 │ return Children.map(children, child => { · ──────────── 88 │ if (typeof child !== 'string') return child ╰──── help: `React.Children` is uncommon and leads to fragile React components. note: https://react.dev/reference/react/Children#alternatives Found 0 warnings and 7 errors. Finished in 226ms on 1555 files with 1 rules using 8 threads. ``` </details>
6c3527b to
68e6f6f
Compare
# Oxlint ### 🚀 Features - 04a5ce0 oxlint: Support `vite.config.ts` `.lint` field (#20214) (leaysgur) - 1735215 linter: Implement `react/no-clone-element` rule. (#20129) (connorshea) - 68e6f6f linter: Implement `react/no-react-children` rule. (#20104) (connorshea) - fe3b32e linter/plugins: Add `oxlint-plugin-eslint` package (#20009) (overlookmotel) ### 🐛 Bug Fixes - 05f6a09 linter/no-inline-comments: Deserialize rule options with serde (#20207) (camc314) - c7eb09d linter/default-case: Deserialize rule options with serde (#20206) (camc314) - f85e16c linter/plugins: Fix types for visitor compilation (#20203) (overlookmotel) - 44e24e0 linter/exhaustive-deps: Ignore type-only typeof deps (#20201) (camc314) - 0b04998 linter/no-fallthrough: Deserialize rule options with serde (#20192) (camc314) - a1031cb linter/new-cap: Deserialize rule options with serde (#20161) (camc314) - ad27fd6 linter: Add help messages to import plugin diagnostics (#20158) (John Costa) - 1340307 linter/plugins: Ensure `after` hooks always run (#20167) (overlookmotel) - c4812ec linter/plugins: Reset visitor compilation state if error during compilation (#20166) (overlookmotel) - 887eecc linter/plugins: Add license notice to `oxlint-plugin-eslint` package (#20164) (overlookmotel) - e1713a4 linter/plugins: Include common chunks in `oxlint-plugin-eslint` package (#20163) (overlookmotel) - a9acb2b linter: Check `globals` entry for `no-undef`, only check es2026 globals for `no-extend-native` and `no-constant-binary-expression` (#20089) (Sysix) - 5559f0d linter/no-unused-vars: `reportUsedIgnorePattern` should not report used rest siblings (#20108) (Don Isaac) - de7c0e2 linter/plugins: Correct error message for `markVariableAsUsed` (#20152) (overlookmotel) ### ⚡ Performance - 3a86427 linter/plugins: Pre-populate cache of `EnterExit` objects at startup (#20194) (overlookmotel) - d243391 linter/plugins: Replace arrays with `Uint8Array`s (#20190) (overlookmotel) - 8742f8b linter/plugins: Pre-populate cache of `VisitProp` objects (#20189) (overlookmotel) - 3061acb linter/plugins: Pre-populate cache of `EnterExit` objects (#20187) (overlookmotel) - c73912b linter/plugins: Free visit functions earlier (#20186) (overlookmotel) - d9f8ff4 linter/plugins: Faster reset of visitor state (#20185) (overlookmotel) - 42aff15 oxlint/lsp: Avoid computing diagnostics for non invoked code actions requests (#20080) (Sysix) ### 📚 Documentation - a080650 linter/plugins: Fix documentation of visitor compilation (#20202) (overlookmotel) - 542a04a linter: Add a link to the cyclomatic complexity Wikipedia article in `eslint/complexity` (#20174) (connorshea) # Oxfmt ### 🚀 Features - 95943aa oxfmt: Support `vite.config.*` `.fmt` field (#20197) (leaysgur) - 172fc07 oxfmt: .js/.ts config file support (#20135) (leaysgur) ### 🐛 Bug Fixes - e483569 oxfmt: Avoid double-escaping in css-in-js (#20211) (leaysgur) Co-authored-by: leaysgur <6259812+leaysgur@users.noreply.github.com>
AI Disclosure: I used Claude Code for the implementation of the rule's
runandis_imported_from_reactfunctions. All other code (diagnostics, tests, rule docs, etc.) were written by me.Fixes #20015. Part of #1022, I guess?
This rule is based on the following rules from
@eslint-react/eslint-plugin:@eslint-react/no-children-count@eslint-react/no-children-for-each@eslint-react/no-children-map@eslint-react/no-children-only@eslint-react/no-children-to-arrayThe goal of the rule is to prevent all usage of
React.Children, which is discouraged by the React docs.Examples of rule violations:
A few things to note, in terms of what this rule intentionally does not catch right now:
This rule will not catch CommonJS imports like
const { Children } = require('react');. It didn't really seem worth the trouble.This does not catch alias imports like
import { Children as ReactChildren } from 'react';. I considered adding that case, but it would make the rule more complex for an edge case, and I wasn't sure that was really worthwhile. The original rule(s) from@eslint-react/eslint-pluginalso didn't handle this.There are a few other ways to get around this, like destructuring from
Reactafter importing it:I don't know that that is worth protecting against? We can always add support for complex cases like that in subsequent PRs if desired.
I tried to keep it as simple as possible, which means there will be some holes like the above, but it will be much easier to review and validate as a result, and the above patterns are generally uncommon anyway.
ecosystem-ci run, no new failures (not that any are even really possible, given this isn't a default rule / doesn't have a fixer): https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/22804771151
For validation purposes, it does add 7 new errors to the bsky results, before:
after:
Running the rule on the bsky codebase, all 7 results are correctly-detected violations:
oxlint results with this rule