Skip to content

feat: add no-useless-fragment #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export default [
| [jsx/no-missing-key](packages/eslint-plugin-jsx/src/rules/no-missing-key.md) | require `key` prop when rendering list |
| [jsx/no-misused-comment-in-textnode](packages/eslint-plugin-jsx/src/rules/no-misused-comment-in-textnode.md) | disallow comments from being inserted as text nodes |
| [jsx/no-script-url](packages/eslint-plugin-jsx/src/rules/no-script-url.md) | disallow `javascript:` URLs as JSX event handler prop's value |
| [jsx/no-useless-fragment](packages/eslint-plugin-jsx/src/rules/no-useless-fragment.md) | disallow unnecessary fragments |
| [jsx/prefer-shorthand-boolean](packages/eslint-plugin-jsx/src/rules/prefer-shorthand-boolean.md) | enforce boolean attributes notation in JSX |

### naming-convention
Expand Down Expand Up @@ -156,7 +157,7 @@ export default [
- [x] `jsx/no-script-url`
- [ ] `jsx/no-target-blank`
- [ ] `jsx/no-unknown-property`
- [ ] `jsx/no-useless-fragment`
- [x] `jsx/no-useless-fragment`
- [ ] `jsx/prefer-fragment-syntax`
- [x] `jsx/prefer-shorthand-boolean`
- [x] `naming-convention/filename-extension`
Expand Down
1 change: 1 addition & 0 deletions eslint-doc-generator.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default {
pathRuleList: "README.md",
ruleDocSectionInclude: ["Rule Details"],
ruleDocTitleFormat: "name",
ruleListColumns: ["name", "description"],
ruleListSplit(rules) {
const record = rules.reduce<Record<string, RuleNamesAndRules>>((acc, [name, rule]) => {
const title = /^([\w-]+)\/[\w-]+/iu.exec(name)?.[1] ?? defaultTitle;
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin-jsx/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import jsxNoLeakedConditionalRendering from "./rules/no-leaked-conditional-rende
import jsxNoMissingKey from "./rules/no-missing-key";
import jsxNoMisusedCommentInTextNode from "./rules/no-misused-comment-in-textnode";
import jsxNoScriptUrl from "./rules/no-script-url";
import jsxNoUselessFragment from "./rules/no-useless-fragment";
import jsxPreferShorthandJsxBoolean from "./rules/prefer-shorthand-boolean";

export { name } from "../package.json";
Expand All @@ -18,5 +19,6 @@ export const rules = {
"no-missing-key": jsxNoMissingKey,
"no-misused-comment-in-textnode": jsxNoMisusedCommentInTextNode,
"no-script-url": jsxNoScriptUrl,
"no-useless-fragment": jsxNoUselessFragment,
"prefer-shorthand-boolean": jsxPreferShorthandJsxBoolean,
} as const;
83 changes: 83 additions & 0 deletions packages/eslint-plugin-jsx/src/rules/no-useless-fragment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# jsx/no-useless-fragment

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a [keyed fragment](https://react.dev/reference/react/Fragment#caveats).

## Rule Details

### ❌ Incorrect

```tsx
<>{foo}</>

<><Foo /></>

<p><>foo</></p>

<></>

<Fragment>foo</Fragment>

<React.Fragment>foo</React.Fragment>

<section>
<>
<div />
<div />
</>
</section>

{showFullName ? <>{fullName}</> : <>{firstName}</>}
```

### ✅ Correct

```tsx
{foo}

<Foo />

<>
<Foo />
<Bar />
</>

<>foo {bar}</>

<> {foo}</>

const cat = <>meow</>

<SomeComponent>
<>
<div />
<div />
</>
</SomeComponent>

<Fragment key={item.id}>{item.value}</Fragment>

{showFullName ? fullName : firstName}
```

## Rule Options

### `allowExpressions`

When `true` single expressions in a fragment will be allowed. 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.

Examples of **correct** code for the rule, when `"allowExpressions"` is `true`:

```jsx
<>{foo}</>

<>
{foo}
</>
```
222 changes: 222 additions & 0 deletions packages/eslint-plugin-jsx/src/rules/no-useless-fragment.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
import { allValid } from "@eslint-react/shared";
import { AST_NODE_TYPES } from "@typescript-eslint/types";

import RuleTester, { getFixturesRootDir } from "../../../../test/rule-tester";
import rule, { RULE_NAME } from "./no-useless-fragment";

const rootDir = getFixturesRootDir();

const ruleTester = new RuleTester({
parser: "@typescript-eslint/parser",
parserOptions: {
ecmaFeatures: {
jsx: true,
},
ecmaVersion: 2021,
sourceType: "module",
project: "./tsconfig.json",
tsconfigRootDir: rootDir,
},
});

ruleTester.run(RULE_NAME, rule, {
valid: [
...allValid,
"<><Foo /><Bar /></>",
"<>foo<div /></>",
"<> <div /></>",
'<>{"moo"} </>',
"<NotFragment />",
"<React.NotFragment />",
"<NotReact.Fragment />",
"<Foo><><div /><div /></></Foo>",
'<div p={<>{"a"}{"b"}</>} />',
"<Fragment key={item.id}>{item.value}</Fragment>",
"<Fooo content={<>eeee ee eeeeeee eeeeeeee</>} />",
"<>{foos.map(foo => foo)}</>",
{
code: "<>{moo}</>",
options: [{ allowExpressions: true }],
},
{
code: `
<>
{moo}
</>
`,
options: [{ allowExpressions: true }],
},
],
invalid: [
{
code: "<></>",
output: null,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: "<>{}</>",
output: null,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: "<p>moo<>foo</></p>",
output: "<p>moofoo</p>",
errors: [
{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment },
{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment },
],
},
{
code: "<>{meow}</>",
output: null,
errors: [{ messageId: "NeedsMoreChildren" }],
},
{
code: "<p><>{meow}</></p>",
output: "<p>{meow}</p>",
errors: [
{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment },
{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment },
],
},
{
code: "<><div/></>",
output: "<div/>",
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: `
<>
<div/>
</>
`,
output: `
<div/>
`,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: "<Fragment />",
output: null,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXElement }],
},
{
code: `
<React.Fragment>
<Foo />
</React.Fragment>
`,
output: `
<Foo />
`,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXElement }],
},
{
code: `
<SomeReact.SomeFragment>
{foo}
</SomeReact.SomeFragment>
`,
output: null,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXElement }],
settings: {
react: {
pragma: "SomeReact",
fragment: "SomeFragment",
},
},
},
{
// Not safe to fix this case because `Eeee` might require child be ReactElement
code: "<Eeee><>foo</></Eeee>",
output: null,
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: "<div><>foo</></div>",
output: "<div>foo</div>",
errors: [
{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment },
{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment },
],
},
{
code: '<div><>{"a"}{"b"}</></div>',
output: '<div>{"a"}{"b"}</div>',
errors: [{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: `
<section>
<Eeee />
<Eeee />
<>{"a"}{"b"}</>
</section>`,
output: `
<section>
<Eeee />
<Eeee />
{"a"}{"b"}
</section>`,
errors: [{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: '<div><Fragment>{"a"}{"b"}</Fragment></div>',
output: '<div>{"a"}{"b"}</div>',
errors: [{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXElement }],
},
{
// whitepace tricky case
code: `
<section>
git<>
<b>hub</b>.
</>

git<> <b>hub</b></>
</section>`,
output: `
<section>
git<b>hub</b>.

git <b>hub</b>
</section>`,
errors: [
{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment, line: 3 },
{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment, line: 7 },
],
},
{
code: '<div>a <>{""}{""}</> a</div>',
output: '<div>a {""}{""} a</div>',
errors: [{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXFragment }],
},
{
code: `
const Comp = () => (
<html>
<React.Fragment />
</html>
);
`,
output: `
const Comp = () => (
<html>
${/* dprint-ignore the trailing whitespace here is intentional */ ""}
</html>
);
`,
errors: [
{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXElement, line: 4 },
{ messageId: "ChildOfHtmlElement", type: AST_NODE_TYPES.JSXElement, line: 4 },
],
},
// Ensure allowExpressions still catches expected violations
{
code: "<><Foo>{moo}</Foo></>",
output: "<Foo>{moo}</Foo>",
options: [{ allowExpressions: true }],
errors: [{ messageId: "NeedsMoreChildren", type: AST_NODE_TYPES.JSXFragment }],
},
],
});
Loading