Skip to content

Commit

Permalink
[New] jsx-no-useless-fragments: add option to allow single expressi…
Browse files Browse the repository at this point in the history
…ons in fragments
  • Loading branch information
Matt Darveniza authored and ljharb committed Jun 17, 2021
1 parent 860ebea commit 9f1d618
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 16 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,13 +5,17 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## Unreleased

### Added
* [`jsx-no-useless-fragments`]: add option to allow single expressions in fragments ([#3006][] @mattdarveniza)

### Fixed
* component detection: use `estraverse` to improve component detection ([#2992][] @Wesitos)
* [`destructuring-assignment`], [`no-multi-comp`], [`no-unstable-nested-components`], component detection: improve component detection ([#3001][] @vedadeepta)

### Changed
* [Docs] [`jsx-no-bind`]: updates discussion of refs ([#2998][] @dimitropoulos)

[#3006]: https://github.com/yannickcr/eslint-plugin-react/pull/3006
[#3001]: https://github.com/yannickcr/eslint-plugin-react/pull/3001
[#2998]: https://github.com/yannickcr/eslint-plugin-react/pull/2998
[#2992]: https://github.com/yannickcr/eslint-plugin-react/pull/2992
Expand Down
13 changes: 13 additions & 0 deletions docs/rules/jsx-no-useless-fragment.md
Expand Up @@ -52,3 +52,16 @@ const cat = <>meow</>

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

### `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}</>
```
17 changes: 14 additions & 3 deletions lib/rules/jsx-no-useless-fragment.js
Expand Up @@ -77,6 +77,10 @@ function containsCallExpression(node) {
&& node.expression.type === 'CallExpression';
}

function isFragmentWithSingleExpression(node) {
return node && node.children.length === 1 && node.children[0].type === 'JSXExpressionContainer';
}

module.exports = {
meta: {
type: 'suggestion',
Expand All @@ -88,12 +92,15 @@ module.exports = {
url: docsUrl('jsx-no-useless-fragment')
},
messages: {
NeedsMoreChidren: 'Fragments should contain more than one child - otherwise, there‘s no need for a Fragment at all.',
NeedsMoreChildren: 'Fragments should contain more than one child - otherwise, there‘s no need for a Fragment at all.',
ChildOfHtmlElement: 'Passing a fragment to an HTML element is useless.'
}
},

create(context) {
const config = context.options[0] || {};
const allowExpressions = config.allowExpressions || false;

const reactPragma = pragmaUtil.getFromContext(context);
const fragmentPragma = pragmaUtil.getFragmentFromContext(context);

Expand Down Expand Up @@ -198,10 +205,14 @@ module.exports = {
return;
}

if (hasLessThanTwoChildren(node) && !isFragmentWithOnlyTextAndIsNotChild(node)) {
if (
hasLessThanTwoChildren(node)
&& !isFragmentWithOnlyTextAndIsNotChild(node)
&& !(allowExpressions && isFragmentWithSingleExpression(node))
) {
context.report({
node,
messageId: 'NeedsMoreChidren',
messageId: 'NeedsMoreChildren',
fix: getFix(node)
});
}
Expand Down
39 changes: 26 additions & 13 deletions tests/lib/rules/jsx-no-useless-fragment.js
Expand Up @@ -67,43 +67,48 @@ ruleTester.run('jsx-no-useless-fragment', rule, {
{
code: '<>{foos.map(foo => foo)}</>',
parser: parsers.BABEL_ESLINT
},
{
code: '<>{moo}</>',
parser: parsers.BABEL_ESLINT,
options: [{allowExpressions: true}]
}
],
invalid: [
{
code: '<></>',
output: null,
errors: [{messageId: 'NeedsMoreChidren', type: 'JSXFragment'}],
errors: [{messageId: 'NeedsMoreChildren', type: 'JSXFragment'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<>{}</>',
output: null,
errors: [{messageId: 'NeedsMoreChidren', type: 'JSXFragment'}],
errors: [{messageId: 'NeedsMoreChildren', type: 'JSXFragment'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<p>moo<>foo</></p>',
output: '<p>moofoo</p>',
errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}],
errors: [{messageId: 'NeedsMoreChildren'}, {messageId: 'ChildOfHtmlElement'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<>{meow}</>',
output: null,
errors: [{messageId: 'NeedsMoreChidren'}],
errors: [{messageId: 'NeedsMoreChildren'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<p><>{meow}</></p>',
output: '<p>{meow}</p>',
errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}],
errors: [{messageId: 'NeedsMoreChildren'}, {messageId: 'ChildOfHtmlElement'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<><div/></>',
output: '<div/>',
errors: [{messageId: 'NeedsMoreChidren'}],
errors: [{messageId: 'NeedsMoreChildren'}],
parser: parsers.BABEL_ESLINT
},
{
Expand All @@ -115,12 +120,12 @@ ruleTester.run('jsx-no-useless-fragment', rule, {
output: `
<div/>
`,
errors: [{messageId: 'NeedsMoreChidren'}],
errors: [{messageId: 'NeedsMoreChildren'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<Fragment />',
errors: [{messageId: 'NeedsMoreChidren'}]
errors: [{messageId: 'NeedsMoreChildren'}]
},
{
code: `
Expand All @@ -131,7 +136,7 @@ ruleTester.run('jsx-no-useless-fragment', rule, {
output: `
<Foo />
`,
errors: [{messageId: 'NeedsMoreChidren'}]
errors: [{messageId: 'NeedsMoreChildren'}]
},
{
code: `
Expand All @@ -145,19 +150,19 @@ ruleTester.run('jsx-no-useless-fragment', rule, {
fragment: 'SomeFragment'
}
},
errors: [{messageId: 'NeedsMoreChidren'}]
errors: [{messageId: 'NeedsMoreChildren'}]
},
{
// Not safe to fix this case because `Eeee` might require child be ReactElement
code: '<Eeee><>foo</></Eeee>',
output: null,
errors: [{messageId: 'NeedsMoreChidren'}],
errors: [{messageId: 'NeedsMoreChildren'}],
parser: parsers.BABEL_ESLINT
},
{
code: '<div><>foo</></div>',
output: '<div>foo</div>',
errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}],
errors: [{messageId: 'NeedsMoreChildren'}, {messageId: 'ChildOfHtmlElement'}],
parser: parsers.BABEL_ESLINT
},
{
Expand Down Expand Up @@ -227,7 +232,15 @@ ruleTester.run('jsx-no-useless-fragment', rule, {
</html>
);
`,
errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}]
errors: [{messageId: 'NeedsMoreChildren'}, {messageId: 'ChildOfHtmlElement'}]
},
// Ensure allowExpressions still catches expected violations
{
code: '<><Foo>{moo}</Foo></>',
options: [{allowExpressions: true}],
errors: [{messageId: 'NeedsMoreChildren'}],
output: '<Foo>{moo}</Foo>',
parser: parsers.BABEL_ESLINT
}
]
});

0 comments on commit 9f1d618

Please sign in to comment.