From 9a5a1b74d42bef6f6e44cf76a78434c02824262d Mon Sep 17 00:00:00 2001 From: Jordan Thomson Date: Thu, 10 Oct 2024 17:32:09 +1100 Subject: [PATCH 1/5] add support for `allowExpressions` in no-useless-fragment --- .../src/rules/no-useless-fragment.spec.ts | 26 +++++++++++++ .../src/rules/no-useless-fragment.ts | 38 ++++++++++++++++--- .../pages/docs/rules/no-useless-fragment.mdx | 4 +- 3 files changed, 60 insertions(+), 8 deletions(-) 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 c2e8b15e9..a506f20b8 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 @@ -118,6 +118,24 @@ ruleTester.run(RULE_NAME, rule, { code: /* tsx */ `<>{moo}`, errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], }, + { + code: /* tsx */ `<>{moo}`, + options: [{ allowExpressions: false }], + errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + }, + { + code: /* tsx */ `<>{moo}`, + options: [{ allowExpressions: false }], + errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + }, + { + code: /* tsx */ `<>{moo}`, + options: [{ allowExpressions: false }], + errors: [{ type: AST_NODE_TYPES.JSXElement, messageId: "noUselessFragment" }, { + type: AST_NODE_TYPES.JSXFragment, + messageId: "noUselessFragment", + }], + }, ], valid: [ ...allValid, @@ -178,5 +196,13 @@ ruleTester.run(RULE_NAME, rule, { }, }, }, + { + code: /* tsx */ `{foo}`, + options: [{ allowExpressions: false }], + }, + { + code: /* tsx */ `} />`, + options: [{ allowExpressions: false }], + }, ], }); 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 b55e2ebf7..8bbba9b11 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 @@ -18,6 +18,7 @@ export type MessageID = function check( node: TSESTree.JSXElement | TSESTree.JSXFragment, context: RuleContext, + allowExpressions: boolean, ) { const initialScope = context.sourceCode.getScope(node); if (JSX.isKeyedElement(node, initialScope)) return; @@ -27,6 +28,14 @@ function check( const [firstChildren] = node.children; // ee eeee eeee ...} /> if (node.children.length === 1 && JSX.isLiteral(firstChildren) && !isChildren) return; + if (!allowExpressions && isChildren) { + // <>hello, world + return context.report({ messageId: "noUselessFragment", node }); + } else if (!allowExpressions && !isChildren && node.children.length === 1) { + // const foo = <>{children}; + // return <>{children}; + return context.report({ messageId: "noUselessFragment", node }); + } const nonPaddingChildren = node.children.filter((child) => !JSX.isPaddingSpaces(child)); if (nonPaddingChildren.length > 1) return; if (nonPaddingChildren.length === 0) return context.report({ messageId: "noUselessFragment", node }); @@ -37,7 +46,13 @@ function check( context.report({ messageId: "noUselessFragment", node }); } -export default createRule<[], MessageID>({ +type Options = [ + { + allowExpressions: boolean; + }, +]; + +export default createRule({ meta: { type: "problem", docs: { @@ -47,19 +62,30 @@ export default createRule<[], MessageID>({ noUselessFragment: "A fragment contains less than two children is unnecessary.", noUselessFragmentInBuiltIn: "A fragment placed inside a built-in component is unnecessary.", }, - schema: [], + schema: [{ + type: "object", + properties: { + allowExpressions: { + type: "boolean", + }, + }, + additionalProperties: false, + }], }, + defaultOptions: [{ + allowExpressions: true, + }], name: RULE_NAME, - create(context) { + create(context, [option]) { + const { allowExpressions = true } = option; return { JSXElement(node) { if (!isFragmentElement(node, context)) return; - check(node, context); + check(node, context, allowExpressions); }, JSXFragment(node) { - check(node, context); + check(node, context, allowExpressions); }, }; }, - defaultOptions: [], }); diff --git a/website/pages/docs/rules/no-useless-fragment.mdx b/website/pages/docs/rules/no-useless-fragment.mdx index 5a5ffdc3c..2cdb73fc9 100644 --- a/website/pages/docs/rules/no-useless-fragment.mdx +++ b/website/pages/docs/rules/no-useless-fragment.mdx @@ -67,10 +67,10 @@ const cat = <>meow ## Note -[This rule always allows single expressions in a fragment](https://github.com/Rel1cx/eslint-react/pull/188). This is useful in +[By default, this rule always allows single expressions in a fragment](https://github.com/Rel1cx/eslint-react/pull/188). This is useful in places like Typescript where `string` does not satisfy the expected return type of `JSX.Element`. A common workaround is to wrap the variable holding a string -in a fragment and expression. +in a fragment and expression. To change this behaviour, use the `allowExpressions` option. ### Examples of correct code for single expressions in fragments: From 1fcc82a85cc71eadf55e6646920a275d5d4e8846 Mon Sep 17 00:00:00 2001 From: Jordan Thomson Date: Thu, 10 Oct 2024 18:25:09 +1100 Subject: [PATCH 2/5] covering more cases --- .../src/rules/no-useless-fragment.spec.ts | 5 +++++ .../src/rules/no-useless-fragment.ts | 14 +++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) 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 a506f20b8..d1ab55637 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 @@ -136,6 +136,11 @@ ruleTester.run(RULE_NAME, rule, { messageId: "noUselessFragment", }], }, + { + code: /* tsx */ `baz}/>`, + options: [{ allowExpressions: false }], + errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + }, ], valid: [ ...allValid, 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 8bbba9b11..100945ae5 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 @@ -1,12 +1,12 @@ import * as AST from "@eslint-react/ast"; -import { isFragmentElement } from "@eslint-react/core"; +import {isFragmentElement} from "@eslint-react/core"; import * as JSX from "@eslint-react/jsx"; -import type { RuleContext } from "@eslint-react/types"; -import { AST_NODE_TYPES } from "@typescript-eslint/types"; -import type { TSESTree } from "@typescript-eslint/utils"; -import { isMatching, P } from "ts-pattern"; +import type {RuleContext} from "@eslint-react/types"; +import {AST_NODE_TYPES} from "@typescript-eslint/types"; +import type {TSESTree} from "@typescript-eslint/utils"; +import {isMatching, P} from "ts-pattern"; -import { createRule } from "../utils"; +import {createRule} from "../utils"; export const RULE_NAME = "no-useless-fragment"; @@ -27,7 +27,7 @@ function check( const isChildren = AST.isOneOf([AST_NODE_TYPES.JSXElement, AST_NODE_TYPES.JSXFragment])(node.parent); const [firstChildren] = node.children; // ee eeee eeee ...} /> - if (node.children.length === 1 && JSX.isLiteral(firstChildren) && !isChildren) return; + if (allowExpressions && node.children.length === 1 && JSX.isLiteral(firstChildren) && !isChildren) return; if (!allowExpressions && isChildren) { // <>hello, world return context.report({ messageId: "noUselessFragment", node }); From 78b1636c78a45259e975cb296629c7458bd8dc6b Mon Sep 17 00:00:00 2001 From: Jordan Thomson Date: Thu, 10 Oct 2024 18:35:29 +1100 Subject: [PATCH 3/5] another test --- .../src/rules/no-useless-fragment.spec.ts | 5 +++++ .../src/rules/no-useless-fragment.ts | 12 ++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) 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 d1ab55637..bc1a13f40 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 @@ -141,6 +141,11 @@ ruleTester.run(RULE_NAME, rule, { options: [{ allowExpressions: false }], errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], }, + { + code: /* tsx */ `<>`, + options: [{ allowExpressions: false }], + errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + }, ], valid: [ ...allValid, 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 100945ae5..b43aaa7af 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 @@ -1,12 +1,12 @@ import * as AST from "@eslint-react/ast"; -import {isFragmentElement} from "@eslint-react/core"; +import { isFragmentElement } from "@eslint-react/core"; import * as JSX from "@eslint-react/jsx"; -import type {RuleContext} from "@eslint-react/types"; -import {AST_NODE_TYPES} from "@typescript-eslint/types"; -import type {TSESTree} from "@typescript-eslint/utils"; -import {isMatching, P} from "ts-pattern"; +import type { RuleContext } from "@eslint-react/types"; +import { AST_NODE_TYPES } from "@typescript-eslint/types"; +import type { TSESTree } from "@typescript-eslint/utils"; +import { isMatching, P } from "ts-pattern"; -import {createRule} from "../utils"; +import { createRule } from "../utils"; export const RULE_NAME = "no-useless-fragment"; From 4ad94cfae8e0031d0b456e69eb0fce729bb32090 Mon Sep 17 00:00:00 2001 From: Jordan Thomson Date: Thu, 10 Oct 2024 18:37:48 +1100 Subject: [PATCH 4/5] lint --- .../src/rules/no-useless-fragment.spec.ts | 10 +++++----- .../src/rules/no-useless-fragment.ts | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) 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 bc1a13f40..7e741c0f8 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 @@ -120,31 +120,31 @@ ruleTester.run(RULE_NAME, rule, { }, { code: /* tsx */ `<>{moo}`, - options: [{ allowExpressions: false }], errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + options: [{ allowExpressions: false }], }, { code: /* tsx */ `<>{moo}`, - options: [{ allowExpressions: false }], errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + options: [{ allowExpressions: false }], }, { code: /* tsx */ `<>{moo}`, - options: [{ allowExpressions: false }], errors: [{ type: AST_NODE_TYPES.JSXElement, messageId: "noUselessFragment" }, { type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment", }], + options: [{ allowExpressions: false }], }, { code: /* tsx */ `baz}/>`, - options: [{ allowExpressions: false }], errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + options: [{ allowExpressions: false }], }, { code: /* tsx */ `<>`, - options: [{ allowExpressions: false }], errors: [{ type: AST_NODE_TYPES.JSXFragment, messageId: "noUselessFragment" }], + options: [{ allowExpressions: false }], }, ], valid: [ 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 b43aaa7af..2f3a43ec0 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 @@ -64,17 +64,14 @@ export default createRule({ }, schema: [{ type: "object", + additionalProperties: false, properties: { allowExpressions: { type: "boolean", }, }, - additionalProperties: false, }], }, - defaultOptions: [{ - allowExpressions: true, - }], name: RULE_NAME, create(context, [option]) { const { allowExpressions = true } = option; @@ -88,4 +85,7 @@ export default createRule({ }, }; }, + defaultOptions: [{ + allowExpressions: true, + }], }); From 2049c7c05f0a1fcba4883575f97cade61ab51588 Mon Sep 17 00:00:00 2001 From: Jordan Thomson Date: Thu, 10 Oct 2024 18:49:48 +1100 Subject: [PATCH 5/5] added more docs --- .../pages/docs/rules/no-useless-fragment.mdx | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/website/pages/docs/rules/no-useless-fragment.mdx b/website/pages/docs/rules/no-useless-fragment.mdx index 2cdb73fc9..f4be93f2c 100644 --- a/website/pages/docs/rules/no-useless-fragment.mdx +++ b/website/pages/docs/rules/no-useless-fragment.mdx @@ -80,6 +80,59 @@ in a fragment and expression. To change this behaviour, use the `allowExpression {foo} ``` +## Examples with `allowExpressions: false` + +### Failing + +```tsx +<> + +

<>foo

+ +<> + +baz} /> + +
+ <> +
+
+ +
+ +const cat = <>meow + +<>{children} + +<>{props.children} + +<> {foo} + + + <> +
+
+ + +``` + +### Passing + +```tsx +{foo} + + + +<> + + + + +<>foo {bar} + +{item.value} +``` + ## Further Reading - [React: Fragment](https://react.dev/reference/react/Fragment)