Skip to content
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

[react/jsx-no-useless-fragments] add option to allow single expressions in fragments #3006

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
}
]
});