Skip to content

Commit

Permalink
[New] add jsx-no-useless-fragment rule
Browse files Browse the repository at this point in the history
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
  • Loading branch information
golopot and ljharb committed May 5, 2019
1 parent 66725bc commit 7ccff10
Show file tree
Hide file tree
Showing 7 changed files with 518 additions and 1 deletion.
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) {
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;
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
};

0 comments on commit 7ccff10

Please sign in to comment.