diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.mdx b/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.mdx index 5dba499ce..7b3d6030d 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.mdx +++ b/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.mdx @@ -87,6 +87,7 @@ const cat = <>meow This rule has a single options object with the following property: +- `allowEmptyFragment` (default: `false`): Allow fragments that contain no children. - `allowExpressions` (default: `true`): Allow fragments that contain a single expression child. ## Note diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.spec.ts b/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.spec.ts index b1b3fa80c..d31bcb71b 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.spec.ts +++ b/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.spec.ts @@ -11,18 +11,30 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], output: null, }, + { + code: "<>", + errors: [ + { + type: T.JSXFragment, + messageId: "noUselessFragment", + data: { reason: "contains less than two children" }, + }, + ], + options: [{ allowEmptyFragment: false }], + output: null, + }, { code: "<>{}", errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -34,12 +46,12 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -50,7 +62,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -60,14 +72,9 @@ ruleTester.run(RULE_NAME, rule, { { code: "

<>{meow}

", errors: [ - // { - // type: T.JSXFragment, - // messageId: "uselessFragment", - // data: { reason: "contains less than two children" }, - // }, { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, ], @@ -78,7 +85,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -93,7 +100,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -106,7 +113,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXElement, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -120,7 +127,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXElement, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -128,27 +135,13 @@ ruleTester.run(RULE_NAME, rule, { `, }, - // { - // code: ` - // - // {foo} - // - // `, - // errors: [ - // { - // type: T.JSXElement, - // messageId: "uselessFragment", - // data: { reason: "contains less than two children" }, - // }, - // ], - // }, { // Not safe to fix this case because `Eeee` might require child be ReactElement code: "<>foo", errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -159,12 +152,12 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -175,7 +168,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, ], @@ -191,7 +184,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, ], @@ -207,7 +200,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXElement, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, ], @@ -226,13 +219,13 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, line: 3, }, { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, line: 7, }, @@ -249,7 +242,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, }, ], @@ -266,13 +259,13 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXElement, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "placed inside a host component" }, line: 4, }, { type: T.JSXElement, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, line: 4, }, @@ -291,7 +284,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { type: T.JSXFragment, - messageId: "uselessFragment", + messageId: "noUselessFragment", data: { reason: "contains less than two children" }, }, ], @@ -305,6 +298,10 @@ ruleTester.run(RULE_NAME, rule, { ], valid: [ ...allValid, + { + code: "<>", + options: [{ allowEmptyFragment: true }], + }, "<>", "<>foo
", "<>
", diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts b/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts index cc357233c..d7d6287fc 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts +++ b/packages/plugins/eslint-plugin-react-x/src/rules/no-useless-fragment.ts @@ -4,8 +4,10 @@ import { getJsxAttribute, isJsxFragmentElement, isJsxHostElement, isJsxText } fr import type { RuleContext, RuleFeature } from "@eslint-react/shared"; import { AST_NODE_TYPES as T } from "@typescript-eslint/types"; import type { TSESTree } from "@typescript-eslint/utils"; +import type { JSONSchema4 } from "@typescript-eslint/utils/json-schema"; import type { RuleFixer, RuleListener } from "@typescript-eslint/utils/ts-eslint"; +import type { CamelCase } from "string-ts"; import { createRule } from "../utils"; export const RULE_NAME = "no-useless-fragment"; @@ -15,18 +17,35 @@ export const RULE_FEATURES = [ "CFG", ] as const satisfies RuleFeature[]; -export type MessageID = "uselessFragment"; +export type MessageID = CamelCase; type Options = readonly [ { - allowExpressions: boolean; + allowEmptyFragment?: boolean; + allowExpressions?: boolean; }, ]; const defaultOptions = [{ + allowEmptyFragment: false, allowExpressions: true, }] as const satisfies Options; +const schema = [{ + type: "object", + additionalProperties: false, + properties: { + allowEmptyFragment: { + type: "boolean", + description: "Allow empty fragments", + }, + allowExpressions: { + type: "boolean", + description: "Allow fragments with a single expression child", + }, + }, +}] as const satisfies [JSONSchema4]; + export default createRule({ meta: { type: "problem", @@ -37,18 +56,9 @@ export default createRule({ }, fixable: "code", messages: { - uselessFragment: "A fragment {{reason}} is useless.", + noUselessFragment: "A fragment {{reason}} is useless.", }, - schema: [{ - type: "object", - additionalProperties: false, - properties: { - allowExpressions: { - type: "boolean", - description: "Allow fragments with a single expression child", - }, - }, - }], + schema, }, name: RULE_NAME, create, @@ -56,17 +66,139 @@ export default createRule({ }); export function create(context: RuleContext, [option]: Options): RuleListener { - const { allowExpressions = true } = option; + const { allowEmptyFragment = false, allowExpressions = true } = option; + /** + * Check if a fragment node is useless and should be reported + */ + function checkNode(context: RuleContext, node: TSESTree.JSXElement | TSESTree.JSXFragment) { + // Skip if the fragment has a key prop (indicates it's needed for lists) + if (node.type === T.JSXElement && getJsxAttribute(context, node)("key") != null) { + return; + } + + // Report fragment placed inside a host component (e.g.
<>
) + if (isJsxHostElement(context, node.parent)) { + context.report({ + messageId: "noUselessFragment", + node, + data: { reason: "placed inside a host component" }, + fix: getFix(context, node), + }); + } + + // Report empty fragments (e.g. <>) + if (node.children.length === 0) { + // https://github.com/Rel1cx/eslint-react/issues/1265 + if (allowEmptyFragment) return; + context.report({ + messageId: "noUselessFragment", + node, + data: { reason: "contains less than two children" }, + fix: getFix(context, node), + }); + return; + } + + const isChildElement = AST.isOneOf([T.JSXElement, T.JSXFragment])(node.parent); + + // Handle various fragment cases + switch (true) { + // Allow single text child in attribute value (e.g. content={<>text}) + case allowExpressions + && !isChildElement + && node.children.length === 1 + && isJsxText(node.children.at(0)): { + return; + } + + // Report fragment with single child inside JSX element + case !allowExpressions + && isChildElement: { + context.report({ + messageId: "noUselessFragment", + node, + data: { reason: "contains less than two children" }, + fix: getFix(context, node), + }); + return; + } + + // Report fragment with single child in expressions + case !allowExpressions + && !isChildElement + && node.children.length === 1: { + context.report({ + messageId: "noUselessFragment", + node, + data: { reason: "contains less than two children" }, + fix: getFix(context, node), + }); + return; + } + } + + // Filter out padding spaces to check actual content + const nonPaddingChildren = node.children.filter((child) => !isPaddingSpaces(child)); + const firstNonPaddingChild = nonPaddingChildren.at(0); + + // Report if empty or only has one non-expression child + if ( + nonPaddingChildren.length === 0 + || (nonPaddingChildren.length === 1 && firstNonPaddingChild?.type !== T.JSXExpressionContainer) + ) { + context.report({ + messageId: "noUselessFragment", + node, + data: { reason: "contains less than two children" }, + fix: getFix(context, node), + }); + } + } + + function getFix(context: RuleContext, node: TSESTree.JSXElement | TSESTree.JSXFragment) { + if (!canFix(context, node)) return null; + + return (fixer: RuleFixer) => { + const opener = node.type === T.JSXFragment ? node.openingFragment : node.openingElement; + const closer = node.type === T.JSXFragment ? node.closingFragment : node.closingElement; + + const childrenText = opener.type === T.JSXOpeningElement && opener.selfClosing + ? "" + : context.sourceCode.getText().slice(opener.range[1], closer?.range[0]); + + return fixer.replaceText(node, trimLikeReact(childrenText)); + }; + } + + /** + * Check if it's safe to automatically fix the fragment + */ + function canFix(context: RuleContext, node: TSESTree.JSXElement | TSESTree.JSXFragment) { + // Don't fix fragments inside custom components (might require children to be ReactElement) + if (node.parent.type === T.JSXElement || node.parent.type === T.JSXFragment) { + return isJsxHostElement(context, node.parent); + } + + // Don't fix empty fragments without a JSX parent + if (node.children.length === 0) { + return false; + } + + // Don't fix fragments with text or expressions outside of JSX context + return !node + .children + .some((child) => (isJsxText(child) && !isWhiteSpace(child)) || AST.is(T.JSXExpressionContainer)(child)); + } return { // Check JSX elements that might be fragments JSXElement(node) { if (!isJsxFragmentElement(context, node)) return; - checkNode(context, node, allowExpressions); + checkNode(context, node); }, // Check JSX fragments JSXFragment(node) { - checkNode(context, node, allowExpressions); + checkNode(context, node); }, }; } @@ -101,128 +233,3 @@ function trimLikeReact(text: string) { return text.slice(start, end); } - -/** - * Check if a fragment node is useless and should be reported - */ -function checkNode( - context: RuleContext, - node: TSESTree.JSXElement | TSESTree.JSXFragment, - allowExpressions: boolean, -) { - // Skip if the fragment has a key prop (indicates it's needed for lists) - if (node.type === T.JSXElement && getJsxAttribute(context, node)("key") != null) { - return; - } - - // Report fragment placed inside a host component (e.g.
<>
) - if (isJsxHostElement(context, node.parent)) { - context.report({ - messageId: "uselessFragment", - node, - data: { reason: "placed inside a host component" }, - fix: getFix(context, node), - }); - } - - // Report empty fragments (e.g. <>) - if (node.children.length === 0) { - context.report({ - messageId: "uselessFragment", - node, - data: { reason: "contains less than two children" }, - fix: getFix(context, node), - }); - return; - } - - const isChildElement = AST.isOneOf([T.JSXElement, T.JSXFragment])(node.parent); - - // Handle various fragment cases - switch (true) { - // Allow single text child in attribute value (e.g. content={<>text}) - case allowExpressions - && !isChildElement - && node.children.length === 1 - && isJsxText(node.children.at(0)): { - return; - } - - // Report fragment with single child inside JSX element - case !allowExpressions - && isChildElement: { - context.report({ - messageId: "uselessFragment", - node, - data: { reason: "contains less than two children" }, - fix: getFix(context, node), - }); - return; - } - - // Report fragment with single child in expressions - case !allowExpressions - && !isChildElement - && node.children.length === 1: { - context.report({ - messageId: "uselessFragment", - node, - data: { reason: "contains less than two children" }, - fix: getFix(context, node), - }); - return; - } - } - - // Filter out padding spaces to check actual content - const nonPaddingChildren = node.children.filter((child) => !isPaddingSpaces(child)); - const firstNonPaddingChild = nonPaddingChildren.at(0); - - // Report if empty or only has one non-expression child - if ( - nonPaddingChildren.length === 0 - || (nonPaddingChildren.length === 1 && firstNonPaddingChild?.type !== T.JSXExpressionContainer) - ) { - context.report({ - messageId: "uselessFragment", - node, - data: { reason: "contains less than two children" }, - fix: getFix(context, node), - }); - } -} - -function getFix(context: RuleContext, node: TSESTree.JSXElement | TSESTree.JSXFragment) { - if (!canFix(context, node)) return null; - - return (fixer: RuleFixer) => { - const opener = node.type === T.JSXFragment ? node.openingFragment : node.openingElement; - const closer = node.type === T.JSXFragment ? node.closingFragment : node.closingElement; - - const childrenText = opener.type === T.JSXOpeningElement && opener.selfClosing - ? "" - : context.sourceCode.getText().slice(opener.range[1], closer?.range[0]); - - return fixer.replaceText(node, trimLikeReact(childrenText)); - }; -} - -/** - * Check if it's safe to automatically fix the fragment - */ -function canFix(context: RuleContext, node: TSESTree.JSXElement | TSESTree.JSXFragment) { - // Don't fix fragments inside custom components (might require children to be ReactElement) - if (node.parent.type === T.JSXElement || node.parent.type === T.JSXFragment) { - return isJsxHostElement(context, node.parent); - } - - // Don't fix empty fragments without a JSX parent - if (node.children.length === 0) { - return false; - } - - // Don't fix fragments with text or expressions outside of JSX context - return !node - .children - .some((child) => (isJsxText(child) && !isWhiteSpace(child)) || AST.is(T.JSXExpressionContainer)(child)); -} diff --git a/packages/plugins/eslint-plugin/README.md b/packages/plugins/eslint-plugin/README.md index 506d2a919..2099f9f73 100644 --- a/packages/plugins/eslint-plugin/README.md +++ b/packages/plugins/eslint-plugin/README.md @@ -191,8 +191,8 @@ This project is and will continue to maintain that 90% of the code is written by Contributions are welcome! -Please follow our [contributing guidelines](https://github.com/Rel1cx/eslint-react/tree/main/.github/CONTRIBUTING.md). +Please follow our [contributing guidelines](https://github.com/Rel1cx/eslint-react/tree/stable/.github/CONTRIBUTING.md). ## License -This project is licensed under the MIT License - see the [LICENSE](https://github.com/Rel1cx/eslint-react/tree/main/LICENSE) file for details. +This project is licensed under the MIT License - see the [LICENSE](https://github.com/Rel1cx/eslint-react/tree/stable/LICENSE) file for details.