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

[New] add jsx-no-useless-fragment rule #2261

Merged
merged 1 commit into from Sep 8, 2019
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
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -170,6 +170,7 @@ Enable the rules that you would like to use.
* [react/jsx-no-literals](docs/rules/jsx-no-literals.md): Prevent usage of unwrapped JSX strings
* [react/jsx-no-target-blank](docs/rules/jsx-no-target-blank.md): Prevent usage of unsafe `target='_blank'`
* [react/jsx-no-undef](docs/rules/jsx-no-undef.md): Disallow undeclared variables in JSX
* [react/jsx-no-useless-fragment](docs/rules/jsx-no-useless-fragment.md): Disallow unnescessary fragments (fixable)
* [react/jsx-one-expression-per-line](docs/rules/jsx-one-expression-per-line.md): Limit to one expression per line in JSX
* [react/jsx-curly-brace-presence](docs/rules/jsx-curly-brace-presence.md): Enforce curly braces or disallow unnecessary curly braces in JSX
* [react/jsx-fragments](docs/rules/jsx-fragments.md): Enforce shorthand or standard form for React fragments
Expand Down
54 changes: 54 additions & 0 deletions docs/rules/jsx-no-useless-fragment.md
@@ -0,0 +1,54 @@
# Disallow unnecessary fragments (react/jsx-no-useless-fragment)

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://reactjs.org/docs/fragments.html#keyed-fragments).

**Fixable:** This rule is sometimes automatically fixable using the `--fix` flag on the command line.

## Rule Details

The following patterns are considered warnings:

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

<><Foo /></>

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

<></>

<Fragment>foo</Fragment>

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

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

The following patterns are **not** considered warnings:

```jsx
<>
<Foo />
<Bar />
</>

<>foo {bar}</>

<> {foo}</>

const cat = <>meow</>

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

<Fragment key={item.id}>{item.value}</Fragment>
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -35,6 +35,7 @@ const allRules = {
'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'),
'jsx-no-literals': require('./lib/rules/jsx-no-literals'),
'jsx-no-target-blank': require('./lib/rules/jsx-no-target-blank'),
'jsx-no-useless-fragment': require('./lib/rules/jsx-no-useless-fragment'),
'jsx-one-expression-per-line': require('./lib/rules/jsx-one-expression-per-line'),
'jsx-no-undef': require('./lib/rules/jsx-no-undef'),
'jsx-curly-brace-presence': require('./lib/rules/jsx-curly-brace-presence'),
Expand Down
210 changes: 210 additions & 0 deletions lib/rules/jsx-no-useless-fragment.js
@@ -0,0 +1,210 @@
/**
* @fileoverview Disallow useless fragments
*/

'use strict';

const pragmaUtil = require('../util/pragma');
const jsxUtil = require('../util/jsx');
const docsUrl = require('../util/docsUrl');

function isJSXText(node) {
return !!node && (node.type === 'JSXText' || node.type === 'Literal');
}

/**
* @param {string} text
* @returns {boolean}
*/
function isOnlyWhitespace(text) {
return text.trim().length === 0;
}

/**
* @param {ASTNode} node
* @returns {boolean}
*/
function isNonspaceJSXTextOrJSXCurly(node) {
return (isJSXText(node) && !isOnlyWhitespace(node.raw)) || node.type === 'JSXExpressionContainer';
}

/**
* Somehow fragment like this is useful: <Foo content={<>ee eeee eeee ...</>} />
* @param {ASTNode} node
* @returns {boolean}
*/
function isFragmentWithOnlyTextAndIsNotChild(node) {
return node.children.length === 1 &&
isJSXText(node.children[0]) &&
!(node.parent.type === 'JSXElement' || node.parent.type === 'JSXFragment');
}

/**
* @param {string} text
* @returns {string}
*/
function trimLikeReact(text) {
const leadingSpaces = /^\s*/.exec(text)[0];
const trailingSpaces = /\s*$/.exec(text)[0];

const start = leadingSpaces.includes('\n') ? leadingSpaces.length : 0;
const end = trailingSpaces.includes('\n') ? text.length - trailingSpaces.length : text.length;

return text.slice(start, end);
}

/**
* Test if node is like `<Fragment key={_}>_</Fragment>`
* @param {JSXElement} node
* @returns {boolean}
*/
function isKeyedElement(node) {
return node.type === 'JSXElement' &&
node.openingElement.attributes &&
node.openingElement.attributes.some(jsxUtil.isJSXAttributeKey);
}

module.exports = {
meta: {
type: 'suggestion',
fixable: 'code',
docs: {
description: 'Disallow unnecessary fragments',
category: 'Possible Errors',
recommended: false,
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.',
ChildOfHtmlElement: 'Passing a fragment to an HTML element is useless.'
}
},

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

/**
* Test whether a node is an padding spaces trimmed by react runtime.
* @param {ASTNode} node
* @returns {boolean}
*/
function isPaddingSpaces(node) {
return isJSXText(node) &&
isOnlyWhitespace(node.raw) &&
node.raw.includes('\n');
}

/**
* Test whether a JSXElement has less than two children, excluding paddings spaces.
* @param {JSXElement|JSXFragment} node
* @returns {boolean}
*/
function hasLessThanTwoChildren(node) {
if (!node || !node.children || node.children.length < 2) {
return true;
}

return (
node.children.length -
(+isPaddingSpaces(node.children[0])) -
(+isPaddingSpaces(node.children[node.children.length - 1]))
) < 2;
}

/**
* @param {JSXElement|JSXFragment} node
* @returns {boolean}
*/
function isChildOfHtmlElement(node) {
return node.parent.type === 'JSXElement' &&
node.parent.openingElement.name.type === 'JSXIdentifier' &&
/^[a-z]+$/.test(node.parent.openingElement.name.name);
}

/**
* @param {JSXElement|JSXFragment} node
* @return {boolean}
*/
function isChildOfComponentElement(node) {
return node.parent.type === 'JSXElement' &&
!isChildOfHtmlElement(node) &&
!jsxUtil.isFragment(node.parent, reactPragma, fragmentPragma);
}

/**
* @param {ASTNode} node
* @returns {boolean}
*/
function canFix(node) {
// Not safe to fix fragments without a jsx parent.
if (!(node.parent.type === 'JSXElement' || node.parent.type === 'JSXFragment')) {
// const a = <></>
if (node.children.length === 0) {
ljharb marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

// const a = <>cat {meow}</>
if (node.children.some(isNonspaceJSXTextOrJSXCurly)) {
return false;
}
}

// Not safe to fix `<Eeee><>foo</></Eeee>` because `Eeee` might require its children be a ReactElement.
if (isChildOfComponentElement(node)) {
return false;
}

return true;
}

/**
* @param {ASTNode} node
* @returns {Function | undefined}
*/
function getFix(node) {
if (!canFix(node)) {
return undefined;
}

return function fix(fixer) {
const opener = node.type === 'JSXFragment' ? node.openingFragment : node.openingElement;
ljharb marked this conversation as resolved.
Show resolved Hide resolved
const closer = node.type === 'JSXFragment' ? node.closingFragment : node.closingElement;
const childrenText = context.getSourceCode().getText().slice(opener.range[1], closer.range[0]);

return fixer.replaceText(node, trimLikeReact(childrenText));
};
}

function checkNode(node) {
if (isKeyedElement(node)) {
return;
}

if (hasLessThanTwoChildren(node) && !isFragmentWithOnlyTextAndIsNotChild(node)) {
context.report({
node,
messageId: 'NeedsMoreChidren',
fix: getFix(node)
});
}

if (isChildOfHtmlElement(node)) {
context.report({
node,
messageId: 'ChildOfHtmlElement',
fix: getFix(node)
});
}
}

return {
JSXElement(node) {
if (jsxUtil.isFragment(node, reactPragma, fragmentPragma)) {
checkNode(node);
}
},
JSXFragment: checkNode
};
}
};
2 changes: 2 additions & 0 deletions lib/types.d.ts
Expand Up @@ -9,6 +9,8 @@ declare global {
type Token = eslint.AST.Token;
type Fixer = eslint.Rule.RuleFixer;
type JSXAttribute = ASTNode;
type JSXElement = ASTNode;
type JSXFragment = ASTNode;
type JSXSpreadAttribute = ASTNode;

interface Context extends eslint.SourceCode {
Expand Down
45 changes: 44 additions & 1 deletion lib/util/jsx.js
Expand Up @@ -26,6 +26,35 @@ function isDOMComponent(node) {
return COMPAT_TAG_REGEX.test(name);
}

/**
* Test whether a JSXElement is a fragment
* @param {JSXElement} node
* @param {string} reactPragma
* @param {string} fragmentPragma
* @returns {boolean}
*/
function isFragment(node, reactPragma, fragmentPragma) {
const name = node.openingElement.name;

// <Fragment>
if (name.type === 'JSXIdentifier' && name.name === fragmentPragma) {
return true;
}

// <React.Fragment>
if (
name.type === 'JSXMemberExpression' &&
name.object.type === 'JSXIdentifier' &&
name.object.name === reactPragma &&
name.property.type === 'JSXIdentifier' &&
name.property.name === fragmentPragma
) {
return true;
}

return false;
}

/**
* Checks if a node represents a JSX element or fragment.
* @param {object} node - node to check.
Expand All @@ -35,7 +64,21 @@ function isJSX(node) {
return node && ['JSXElement', 'JSXFragment'].indexOf(node.type) >= 0;
}

/**
* Check if node is like `key={...}` as in `<Foo key={...} />`
* @param {ASTNode} node
* @returns {boolean}
*/
function isJSXAttributeKey(node) {
return node.type === 'JSXAttribute' &&
node.name &&
node.name.type === 'JSXIdentifier' &&
node.name.name === 'key';
}

module.exports = {
isDOMComponent,
isJSX
isFragment,
isJSX,
isJSXAttributeKey
};