diff --git a/.changeset/rn-no-raw-text-wrapper-false-positive.md b/.changeset/rn-no-raw-text-wrapper-false-positive.md new file mode 100644 index 000000000..c426b233d --- /dev/null +++ b/.changeset/rn-no-raw-text-wrapper-false-positive.md @@ -0,0 +1,6 @@ +--- +"oxlint-plugin-react-doctor": patch +"react-doctor": patch +--- + +Fix false positives in `rn-no-raw-text` (#788) for custom components that forward their children into a ``: the in-file wrapper detection now recognizes components that render `{children}` (or `{props.children}`) inside a nested `` (the `{children}` shape), not just components whose returned root is a ``. Detection also handles parenthesized `return (...)` bodies, `memo`/`forwardRef`-wrapped components, fragment roots, conditional and logical returns, early returns inside `if` branches, renamed destructured children (`{ children: content }`), the `` prop form, wrappers that forward through another in-file wrapper, children aliased to a variable or destructured from props in the body, props spreads that carry children (``, ``, ``), class components, and `styled(Text)` / `styled.Text` factories. The rule is also tagged `test-noise`, so it no longer fires in test/story files — raw text rendered through React Native Testing Library never ships to users, and cross-file wrappers (an imported `Test Chip` in a `.test.tsx`) were the main source of unfixable noise there. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.test.ts index 37650e73d..3bbeaf5e7 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.test.ts @@ -78,6 +78,307 @@ describe("react-native/rn-no-raw-text", () => { const App = () => ; `); }); + + it("suppresses a wrapper forwarding children into a nested Text", () => { + expectPass(` + function Chip({ children }) { + return ( + + {children} + + ); + } + const App = () => Test Chip; + `); + }); + + it("suppresses an arrow wrapper forwarding props.children into a nested Text", () => { + expectPass(` + const Badge = (props) => ( + + {props.children} + + ); + const App = () => New; + `); + }); + + it("suppresses a forwardRef/memo wrapper forwarding children into a nested Text", () => { + expectPass(` + import { forwardRef, memo } from "react"; + const Chip = memo( + forwardRef(({ children }, ref) => ( + + {children} + + )), + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper with a conditional return", () => { + expectPass(` + const Chip = ({ children, isLoading }) => + isLoading ? : ( + + {children} + + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper returning a fragment with a nested Text", () => { + expectPass(` + const Chip = ({ children }) => ( + <> + + {children} + + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper with renamed destructured children", () => { + expectPass(` + const Chip = ({ children: content }) => ( + + {content} + + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper using the children prop form on Text", () => { + expectPass(` + const Chip = ({ children }) => ( + + + + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper with a return inside an if branch", () => { + expectPass(` + function Chip({ children, compact }) { + if (compact) { + return {children}; + } + return ( + + {children} + + ); + } + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper that forwards children through another in-file wrapper", () => { + expectPass(` + const Chip = ({ children }) => ( + + {children} + + ); + const Badge = ({ children }) => {children}; + const App = () => New; + `); + }); + + it("suppresses a wrapper that aliases children to a variable", () => { + expectPass(` + function Chip({ children }) { + const content = children; + return ( + + {content} + + ); + } + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper that destructures children from props in the body", () => { + expectPass(` + const Chip = (props) => { + const { children } = props; + return ( + + {children} + + ); + }; + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper spreading props onto a nested Text", () => { + expectPass(` + const Chip = (props) => ( + + + + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a wrapper spreading an object rest that carries children", () => { + expectPass(` + const Chip = ({ style, ...rest }) => ( + + + + ); + const App = () => Test Chip; + `); + }); + + it("still fires when the spread rest excludes children", () => { + expectFail(` + const Chip = ({ children, ...rest }) => ( + + + {children} + + ); + const App = () => Test Chip; + `); + }); + + it("suppresses a class component forwarding this.props.children into a Text", () => { + expectPass(` + class Chip extends React.Component { + render() { + return ( + + {this.props.children} + + ); + } + } + const App = () => Test Chip; + `); + }); + + it("suppresses a styled(Text) factory component", () => { + expectPass(` + const FancyChip = styled(Text)\` + color: red; + \`; + const App = () => Test Chip; + `); + }); + + it("suppresses a styled.Text factory component", () => { + expectPass(` + const FancyCopy = styled.Text({ color: "red" }); + const App = () => Test Chip; + `); + }); + + it("still fires for a styled(View) factory component", () => { + expectFail(` + const Card = styled(View)\` + padding: 4px; + \`; + const App = () => Test Chip; + `); + }); + + it("still fires when one branch renders children outside a Text", () => { + expectFail(` + const Chip = ({ children, inline }) => { + if (inline) return {children}; + return ( + + {children} + + ); + }; + const App = () => Test Chip; + `); + }); + + it("still fires when children comes from an unrelated destructure", () => { + expectFail(` + const Chip = ({ item }) => { + const { children } = item; + return ( + + {children} + + ); + }; + const App = () => Test Chip; + `); + }); + + it("still fires when the nested Text receives an unrelated object's children", () => { + expectFail(` + const Chip = ({ item }) => ( + + {item.children} + + ); + const App = () => Test Chip; + `); + }); + + it("still fires when one branch spreads props onto a non-text element", () => { + expectFail(` + const Chip = (props) => { + if (props.inline) return ; + return ( + + {props.children} + + ); + }; + const App = () => Test Chip; + `); + }); + + it("does not treat a render-prop's Text as the wrapper's own markup", () => { + expectFail(` + const Box = ({ children, renderLabel }) => ( + + {() => {children}} + {children} + + ); + const App = () => Hello; + `); + }); + + it("still fires when the nested Text receives something other than children", () => { + expectFail(` + const Card = ({ title, children }) => ( + + {title} + {children} + + ); + const App = () => Body copy; + `); + }); + }); + + describe("test-noise suppression", () => { + it("does not fire in testlike files", () => { + const result = runRule(rnNoRawText, `const App = () => Hello;`, { + filename: "Chip.test.tsx", + }); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); }); describe("expo universal ui ListItem", () => { diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.ts index 80c9f5449..9523e89ed 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/rn-no-raw-text.ts @@ -96,6 +96,7 @@ export const rnNoRawText = defineRule({ title: "Raw text outside a Text component", requires: ["react-native"], severity: "error", + tags: ["test-noise"], recommendation: "Text outside a `` component crashes on React Native. Wrap it like `{value}`.", create: (context: RuleContext) => { @@ -108,8 +109,9 @@ export const rnNoRawText = defineRule({ // in a WebView as DOM rather than on React Native primitives. let isDomComponentFile = false; - // Auto-detected in-file text wrappers — components whose returned root is - // a real `` (so they forward children into text). Populated from the + // Auto-detected in-file text wrappers — components that forward their + // children into a real `` (either as the returned root or nested + // inside the returned markup). Populated from the // program on first visit so usage anywhere in the file (declared before or // after) is seen. Manual `textComponents` / `rawTextWrapperComponents` // overrides are applied separately in the core diagnostic pipeline diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/utils/collect-text-wrapper-components.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/utils/collect-text-wrapper-components.ts index f23b92055..6616e6c20 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/utils/collect-text-wrapper-components.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-native/utils/collect-text-wrapper-components.ts @@ -2,6 +2,7 @@ import type { EsTreeNode } from "../../../utils/es-tree-node.js"; import type { EsTreeNodeOfType } from "../../../utils/es-tree-node-of-type.js"; import { isNodeOfType } from "../../../utils/is-node-of-type.js"; import { isReactComponentName } from "../../../utils/is-react-component-name.js"; +import { stripParenExpression } from "../../../utils/strip-paren-expression.js"; import { walkAst } from "../../../utils/walk-ast.js"; import { resolveJsxElementName } from "./resolve-jsx-element-name.js"; @@ -15,67 +16,409 @@ const isFunctionNode = (node: EsTreeNode): node is FunctionNode => isNodeOfType(node, "FunctionExpression") || isNodeOfType(node, "FunctionDeclaration"); -// Resolves the tag name of the single JSX element a component returns at the -// top level of its body, or `null` when the component returns a fragment, -// conditional markup, or no JSX. Only the *root* element matters here: a -// wrapper like `({ children }) => {children}` forwards its -// children into that root, so the root's identity decides whether the wrapper -// is text-handling. We deliberately stay shallow (expression body, or a -// `ReturnStatement` at the block's top level) to keep detection high-confidence. -const resolveReturnedRootElementName = (functionNode: FunctionNode): string | null => { +const COMPONENT_WRAPPER_CALLEE_NAMES = new Set(["memo", "forwardRef"]); + +const resolveCalleeName = (callee: EsTreeNode): string | null => { + if (isNodeOfType(callee, "Identifier")) return callee.name; + if (isNodeOfType(callee, "MemberExpression") && isNodeOfType(callee.property, "Identifier")) { + return callee.property.name; + } + return null; +}; + +// Peels `memo(...)` / `forwardRef(...)` / `React.memo(React.forwardRef(...))` +// down to the render function so those wrapped components are analyzed too. +const unwrapComponentDefinition = (node: EsTreeNode): EsTreeNode => { + let current = stripParenExpression(node); + while (isNodeOfType(current, "CallExpression")) { + const calleeName = resolveCalleeName(current.callee); + const firstArgument = current.arguments?.[0]; + if (!calleeName || !COMPONENT_WRAPPER_CALLEE_NAMES.has(calleeName) || !firstArgument) break; + current = stripParenExpression(firstArgument); + } + return current; +}; + +interface ChildrenBindings { + // Identifiers that hold the component's children (`children`, a destructure + // rename, or a later alias like `const content = children`). + childrenNames: Set; + // Identifiers whose `.children` (and whose spread) carries the component's + // children — the props param or an object rest that still includes children. + propsObjectNames: Set; +} + +const resolveChildrenPropertyLocalName = (property: EsTreeNode): string | null => { + if (!isNodeOfType(property, "Property")) return null; + if (!isNodeOfType(property.key, "Identifier") || property.key.name !== "children") return null; + const value = property.value; + if (isNodeOfType(value, "Identifier")) return value.name; + if (isNodeOfType(value, "AssignmentPattern") && isNodeOfType(value.left, "Identifier")) { + return value.left.name; + } + return null; +}; + +// The identifiers the component's children are bound to: `children` for +// `({ children })` and props-object params, the rename in +// `({ children: content })`, plus the props object itself (`props` or an +// object rest that still carries children). +const resolveParamChildrenBindings = (functionNode: FunctionNode): ChildrenBindings => { + const bindings: ChildrenBindings = { + childrenNames: new Set(), + propsObjectNames: new Set(), + }; + const firstParam = functionNode.params?.[0]; + if (!firstParam) return bindings; + if (isNodeOfType(firstParam, "Identifier")) { + bindings.propsObjectNames.add(firstParam.name); + return bindings; + } + if (!isNodeOfType(firstParam, "ObjectPattern")) return bindings; + let didDestructureChildren = false; + let restName: string | null = null; + for (const property of firstParam.properties ?? []) { + if (isNodeOfType(property, "RestElement") && isNodeOfType(property.argument, "Identifier")) { + restName = property.argument.name; + continue; + } + const localName = resolveChildrenPropertyLocalName(property); + if (localName) { + didDestructureChildren = true; + bindings.childrenNames.add(localName); + } + } + if (restName && !didDestructureChildren) bindings.propsObjectNames.add(restName); + return bindings; +}; + +const MAX_CHILDREN_ALIAS_PASSES = 3; + +// The props object itself: a param identifier (or qualifying rest), or +// `this.props` inside a class render method. +const isPropsObjectExpression = ( + expression: EsTreeNode | null | undefined, + bindings: ChildrenBindings, +): boolean => { + if (!expression) return false; + const value = stripParenExpression(expression); + if (isNodeOfType(value, "Identifier")) return bindings.propsObjectNames.has(value.name); + return ( + isNodeOfType(value, "MemberExpression") && + isNodeOfType(value.object, "ThisExpression") && + isNodeOfType(value.property, "Identifier") && + value.property.name === "props" + ); +}; + +const isChildrenValueExpression = ( + expression: EsTreeNode | null | undefined, + bindings: ChildrenBindings, +): boolean => { + if (!expression) return false; + const value = stripParenExpression(expression); + if (isNodeOfType(value, "Identifier")) return bindings.childrenNames.has(value.name); + return ( + isNodeOfType(value, "MemberExpression") && + isNodeOfType(value.property, "Identifier") && + value.property.name === "children" && + isPropsObjectExpression(value.object, bindings) + ); +}; + +// Folds body-level aliases into the bindings: `const content = children`, +// `const { children } = props` (or `this.props`), and re-aliases of aliases +// (bounded passes). +const collectChildrenAliases = (functionNode: FunctionNode, bindings: ChildrenBindings): void => { const { body } = functionNode; - if (!body) return null; + if (!body || !isNodeOfType(body, "BlockStatement")) return; + for (let pass = 0; pass < MAX_CHILDREN_ALIAS_PASSES; pass += 1) { + const sizeBeforePass = bindings.childrenNames.size; + walkAst(body, (node) => { + if (isFunctionNode(node)) return false; + if (!isNodeOfType(node, "VariableDeclarator") || !node.init) return undefined; + if (isNodeOfType(node.id, "Identifier")) { + if (isChildrenValueExpression(node.init, bindings)) { + bindings.childrenNames.add(node.id.name); + } + return undefined; + } + if (isNodeOfType(node.id, "ObjectPattern") && isPropsObjectExpression(node.init, bindings)) { + for (const property of node.id.properties ?? []) { + const localName = resolveChildrenPropertyLocalName(property); + if (localName) bindings.childrenNames.add(localName); + } + } + return undefined; + }); + if (bindings.childrenNames.size === sizeBeforePass) break; + } +}; + +// Collects the JSX roots a value can evaluate to, looking through parentheses, +// ternaries, and `&&` / `||` / `??` chains — e.g. both branches of +// `isLoading ? : {children}`. +const collectJsxRootsFromExpression = (expression: EsTreeNode, roots: EsTreeNode[]): void => { + const value = stripParenExpression(expression); + if (isNodeOfType(value, "JSXElement") || isNodeOfType(value, "JSXFragment")) { + roots.push(value); + return; + } + if (isNodeOfType(value, "ConditionalExpression")) { + if (value.consequent) collectJsxRootsFromExpression(value.consequent, roots); + if (value.alternate) collectJsxRootsFromExpression(value.alternate, roots); + return; + } + if (isNodeOfType(value, "LogicalExpression")) { + if (value.left) collectJsxRootsFromExpression(value.left, roots); + if (value.right) collectJsxRootsFromExpression(value.right, roots); + } +}; + +// Resolves the JSX roots a component can return: the expression body, or the +// arguments of `ReturnStatement`s anywhere in the body (so early returns and +// returns inside `if` branches are seen). +const collectReturnedJsxRoots = (functionNode: FunctionNode): EsTreeNode[] => { + const roots: EsTreeNode[] = []; + const { body } = functionNode; + if (!body) return roots; if (!isNodeOfType(body, "BlockStatement")) { - return isNodeOfType(body, "JSXElement") ? resolveJsxElementName(body.openingElement) : null; + collectJsxRootsFromExpression(body, roots); + return roots; } - for (const statement of body.body) { - if (!isNodeOfType(statement, "ReturnStatement")) continue; - const argument = statement.argument; - if (argument && isNodeOfType(argument, "JSXElement")) { - return resolveJsxElementName(argument.openingElement); + walkAst(body, (node) => { + if (isFunctionNode(node) && node !== functionNode) return false; + if (isNodeOfType(node, "ReturnStatement") && node.argument) { + collectJsxRootsFromExpression(node.argument, roots); + return false; } + return undefined; + }); + return roots; +}; + +const isChildrenForwardingJsxChild = (child: EsTreeNode, bindings: ChildrenBindings): boolean => + isNodeOfType(child, "JSXExpressionContainer") && + isChildrenValueExpression(child.expression, bindings); + +// `children={children}` or a props spread (`{...props}` / `{...this.props}`) +// that carries the component's children onto the element. +const isChildrenForwardingAttribute = ( + attribute: EsTreeNode, + bindings: ChildrenBindings, +): boolean => { + if (isNodeOfType(attribute, "JSXSpreadAttribute")) { + return isPropsObjectExpression(attribute.argument, bindings); + } + return ( + isNodeOfType(attribute, "JSXAttribute") && + isNodeOfType(attribute.name, "JSXIdentifier") && + attribute.name.name === "children" && + isNodeOfType(attribute.value, "JSXExpressionContainer") && + isChildrenValueExpression(attribute.value.expression, bindings) + ); +}; + +// True when somewhere in the returned JSX a text-handling element directly +// receives the component's children — `{children}`, +// `{props.children}`, or `` — where +// the wrapper's raw string children still land inside a `` even though +// the root element isn't one. +const jsxRootForwardsChildrenIntoText = ( + jsxRoot: EsTreeNode, + bindings: ChildrenBindings, + isTextHandlingElement: (elementName: string) => boolean, +): boolean => { + let didForwardIntoText = false; + walkAst(jsxRoot, (node) => { + if (didForwardIntoText || isFunctionNode(node)) return false; + if (!isNodeOfType(node, "JSXElement")) return undefined; + const elementName = resolveJsxElementName(node.openingElement); + if (!elementName || !isTextHandlingElement(elementName)) return; + didForwardIntoText = + (node.children ?? []).some((child) => isChildrenForwardingJsxChild(child, bindings)) || + (node.openingElement.attributes ?? []).some((attribute) => + isChildrenForwardingAttribute(attribute, bindings), + ); + }); + return didForwardIntoText; +}; + +const isMeaningfulJsxChild = (child: EsTreeNode): boolean => + !isNodeOfType(child, "JSXText") || Boolean(child.value?.trim()); + +// True when a non-text element directly receives the component's children +// (`{children}`, or a children-carrying attribute like +// `` with no JSX children to override it) — a return path +// like this would render the wrapper's raw string children outside any +// ``, so the component must not be treated as a safe wrapper even if +// another path forwards into text. +const jsxRootRendersChildrenOutsideText = ( + jsxRoot: EsTreeNode, + bindings: ChildrenBindings, + isTextHandlingElement: (elementName: string) => boolean, +): boolean => { + let didRenderOutsideText = false; + walkAst(jsxRoot, (node) => { + if (didRenderOutsideText || isFunctionNode(node)) return false; + if (!isNodeOfType(node, "JSXElement") && !isNodeOfType(node, "JSXFragment")) { + return undefined; + } + if (isNodeOfType(node, "JSXElement")) { + const elementName = resolveJsxElementName(node.openingElement); + if (elementName && isTextHandlingElement(elementName)) return false; + const hasJsxChildren = (node.children ?? []).some(isMeaningfulJsxChild); + if ( + !hasJsxChildren && + (node.openingElement.attributes ?? []).some((attribute) => + isChildrenForwardingAttribute(attribute, bindings), + ) + ) { + didRenderOutsideText = true; + return undefined; + } + } + didRenderOutsideText = (node.children ?? []).some((child) => + isChildrenForwardingJsxChild(child, bindings), + ); + return undefined; + }); + return didRenderOutsideText; +}; + +// Resolves a styled-component factory back to its base element name — +// `styled(Text)`…``, `styled.Text`…``, `styled(Text)({})`, and +// `styled(Text).attrs(…)`…`` all resolve to "Text". +const resolveStyledFactoryBaseName = (definitionNode: EsTreeNode): string | null => { + let current: EsTreeNode | null = stripParenExpression(definitionNode); + while (current) { + if (isNodeOfType(current, "TaggedTemplateExpression")) { + current = stripParenExpression(current.tag); + continue; + } + if (isNodeOfType(current, "CallExpression")) { + const callee = stripParenExpression(current.callee); + if (isNodeOfType(callee, "Identifier") && callee.name === "styled") { + const baseArgument = current.arguments?.[0]; + if (!baseArgument) return null; + const base = stripParenExpression(baseArgument); + return isNodeOfType(base, "Identifier") ? base.name : null; + } + current = callee; + continue; + } + if (isNodeOfType(current, "MemberExpression")) { + if ( + isNodeOfType(current.object, "Identifier") && + current.object.name === "styled" && + isNodeOfType(current.property, "Identifier") + ) { + return current.property.name; + } + current = stripParenExpression(current.object); + continue; + } + return null; + } + return null; +}; + +// The render function of a class component (`class Chip extends Component { +// render() { … } }`), or `null` when the node isn't a class or has no render. +const resolveClassRenderFunction = (classNode: EsTreeNode): FunctionNode | null => { + if (!isNodeOfType(classNode, "ClassDeclaration") && !isNodeOfType(classNode, "ClassExpression")) { + return null; + } + for (const member of classNode.body?.body ?? []) { + if (!isNodeOfType(member, "MethodDefinition")) continue; + if (!isNodeOfType(member.key, "Identifier") || member.key.name !== "render") continue; + return member.value && isFunctionNode(member.value) ? member.value : null; } return null; }; -// Records a component declaration when its name is PascalCase and its returned -// root element is text-handling. Both `const Label = (...) => ` -// (variable declarator) and `function Label(...) { return }` -// (declaration) are covered. +// Records a component declaration when its name is PascalCase and it forwards +// its children into a text-handling element — either a returned root that is +// itself text-handling (`const Label = (...) => `) or a nested +// `{children}` inside any returned markup. const recordWrapperFromDeclaration = ( componentName: string | null, - functionNode: EsTreeNode | null | undefined, - isTextHandlingRoot: (elementName: string) => boolean, + definitionNode: EsTreeNode | null | undefined, + isTextHandlingElement: (elementName: string) => boolean, wrappers: Set, ): void => { if (!componentName || !isReactComponentName(componentName)) return; - if (!functionNode || !isFunctionNode(functionNode)) return; - const rootName = resolveReturnedRootElementName(functionNode); - if (rootName && isTextHandlingRoot(rootName)) wrappers.add(componentName); + if (wrappers.has(componentName)) return; + if (!definitionNode) return; + const unwrapped = unwrapComponentDefinition(definitionNode); + const styledBaseName = resolveStyledFactoryBaseName(unwrapped); + if (styledBaseName && isTextHandlingElement(styledBaseName)) { + wrappers.add(componentName); + return; + } + const functionNode = + resolveClassRenderFunction(unwrapped) ?? (isFunctionNode(unwrapped) ? unwrapped : null); + if (!functionNode) return; + const bindings = resolveParamChildrenBindings(functionNode); + collectChildrenAliases(functionNode, bindings); + const jsxRoots = collectReturnedJsxRoots(functionNode); + if ( + jsxRoots.some((jsxRoot) => + jsxRootRendersChildrenOutsideText(jsxRoot, bindings, isTextHandlingElement), + ) + ) { + return; + } + for (const jsxRoot of jsxRoots) { + if (isNodeOfType(jsxRoot, "JSXElement")) { + const rootName = resolveJsxElementName(jsxRoot.openingElement); + if (rootName && isTextHandlingElement(rootName)) { + wrappers.add(componentName); + return; + } + } + if (jsxRootForwardsChildrenIntoText(jsxRoot, bindings, isTextHandlingElement)) { + wrappers.add(componentName); + return; + } + } }; -// Walks a program once and returns the names of in-file components that -// forward their children into a text-handling root element. These behave like -// configured `rawTextWrapperComponents`: raw text inside them is safe only when -// the children are string-only (mixed children still get reported), since the -// wrapper is assumed to forward `children` into a single ``. +const MAX_TRANSITIVE_WRAPPER_PASSES = 3; + +// Walks a program and returns the names of in-file components that forward +// their children into a text-handling element. These behave like configured +// `rawTextWrapperComponents`: raw text inside them is safe only when the +// children are string-only (mixed children still get reported), since the +// wrapper is assumed to forward `children` into a single ``. Repeats the +// walk a bounded number of times so wrappers-of-wrappers +// (`const Badge = ({ children }) => {children}`) are detected. export const collectTextWrapperComponents = ( programNode: EsTreeNode, isTextHandlingRoot: (elementName: string) => boolean, ): ReadonlySet => { const wrappers = new Set(); + const isTextHandlingElement = (elementName: string): boolean => + isTextHandlingRoot(elementName) || wrappers.has(elementName); - walkAst(programNode, (node) => { - if (isNodeOfType(node, "VariableDeclarator")) { - const componentName = node.id && isNodeOfType(node.id, "Identifier") ? node.id.name : null; - recordWrapperFromDeclaration(componentName, node.init, isTextHandlingRoot, wrappers); - } else if (isNodeOfType(node, "FunctionDeclaration")) { - const componentName = node.id && isNodeOfType(node.id, "Identifier") ? node.id.name : null; - recordWrapperFromDeclaration(componentName, node, isTextHandlingRoot, wrappers); - } - }); + for (let pass = 0; pass < MAX_TRANSITIVE_WRAPPER_PASSES; pass += 1) { + const sizeBeforePass = wrappers.size; + walkAst(programNode, (node) => { + if (isNodeOfType(node, "VariableDeclarator")) { + const componentName = node.id && isNodeOfType(node.id, "Identifier") ? node.id.name : null; + recordWrapperFromDeclaration(componentName, node.init, isTextHandlingElement, wrappers); + } else if ( + isNodeOfType(node, "FunctionDeclaration") || + isNodeOfType(node, "ClassDeclaration") + ) { + const componentName = node.id && isNodeOfType(node.id, "Identifier") ? node.id.name : null; + recordWrapperFromDeclaration(componentName, node, isTextHandlingElement, wrappers); + } + }); + if (wrappers.size === sizeBeforePass) break; + } return wrappers; }; diff --git a/packages/react-doctor/tests/rule-tag-registration.test.ts b/packages/react-doctor/tests/rule-tag-registration.test.ts index 829707d74..32c8a5f14 100644 --- a/packages/react-doctor/tests/rule-tag-registration.test.ts +++ b/packages/react-doctor/tests/rule-tag-registration.test.ts @@ -48,11 +48,13 @@ describe("rule tag registration", () => { }); it("preserves rule-authored tags alongside bucket auto-tags (e.g. test-noise stays on react-native rules that opted in)", () => { - // `rn-no-raw-text` is in the react-native bucket; its only auto-tag - // is "react-native". `no-react19-deprecated-apis` is in architecture - // and authors both "test-noise" and "migration-hint" — no auto-tag - // overwrites those. - expect(getRuleTags("rn-no-raw-text")).toEqual(["react-native"]); + // `rn-no-raw-text` is in the react-native bucket and authors + // "test-noise"; the bucket auto-tag is added alongside it. + // `no-react19-deprecated-apis` is in architecture and authors both + // "test-noise" and "migration-hint" — no auto-tag overwrites those. + const rnNoRawTextTags = getRuleTags("rn-no-raw-text"); + expect(rnNoRawTextTags).toContain("react-native"); + expect(rnNoRawTextTags).toContain("test-noise"); const migrationHintTags = getRuleTags("no-react19-deprecated-apis"); expect(migrationHintTags).toContain("test-noise"); expect(migrationHintTags).toContain("migration-hint");