Skip to content

Commit

Permalink
[New] add jsx-no-leaked-render
Browse files Browse the repository at this point in the history
  • Loading branch information
Belco90 authored and ljharb committed Apr 26, 2022
1 parent fdfbc60 commit 6b7b191
Show file tree
Hide file tree
Showing 6 changed files with 1,030 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`no-unknown-property`]: Allow crossOrigin on image tag (SVG) ([#3251][] @zpao)
* [`jsx-tag-spacing`]: Add `multiline-always` option ([#3260][] @Nokel81)
* [`function-component-definition`]: replace `var` by `const` in certain situations ([#3248][] @JohnBerd @SimeonC)
* add [`jsx-no-leaked-render`] ([#3203][] @Belco90)

### Fixed
* [`hook-use-state`]: Allow UPPERCASE setState setter prefixes ([#3244][] @duncanbeevers)
Expand Down Expand Up @@ -3685,6 +3686,7 @@ If you're still not using React 15 you can keep the old behavior by setting the
[`jsx-no-comment-textnodes`]: docs/rules/jsx-no-comment-textnodes.md
[`jsx-no-constructed-context-values`]: docs/rules/jsx-no-constructed-context-values.md
[`jsx-no-duplicate-props`]: docs/rules/jsx-no-duplicate-props.md
[`jsx-no-leaked-render`]: docs/rules/jsx-no-leaked-render.md
[`jsx-no-literals`]: docs/rules/jsx-no-literals.md
[`jsx-no-script-url`]: docs/rules/jsx-no-script-url.md
[`jsx-no-target-blank`]: docs/rules/jsx-no-target-blank.md
Expand Down
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -202,6 +202,7 @@ Enable the rules that you would like to use.
|| | [react/jsx-no-comment-textnodes](docs/rules/jsx-no-comment-textnodes.md) | Comments inside children section of tag should be placed inside braces |
| | | [react/jsx-no-constructed-context-values](docs/rules/jsx-no-constructed-context-values.md) | Prevents JSX context provider values from taking values that will cause needless rerenders. |
|| | [react/jsx-no-duplicate-props](docs/rules/jsx-no-duplicate-props.md) | Enforce no duplicate props |
| | 🔧 | [react/jsx-no-leaked-render](docs/rules/jsx-no-leaked-render.md) | Prevent problematic leaked values from being rendered |
| | | [react/jsx-no-literals](docs/rules/jsx-no-literals.md) | Prevent using string literals in React component definition |
| | | [react/jsx-no-script-url](docs/rules/jsx-no-script-url.md) | Forbid `javascript:` URLs |
|| 🔧 | [react/jsx-no-target-blank](docs/rules/jsx-no-target-blank.md) | Forbid `target="_blank"` attribute without `rel="noreferrer"` |
Expand Down
208 changes: 208 additions & 0 deletions docs/rules/jsx-no-leaked-render.md
@@ -0,0 +1,208 @@
# Prevent problematic leaked values from being rendered (react/jsx-no-leaked-render)

Using the `&&` operator to render some element conditionally in JSX can cause unexpected values being rendered, or even crashing the rendering.


## Rule Details

This rule aims to prevent dangerous leaked values from being rendered since they can cause unexpected values reaching the final DOM or even crashing your render method.

In React, you might end up rendering unexpected values like `0` or `NaN`. In React Native, your render method will crash if you render `0`, `''`, or `NaN`:

```jsx
const Example = () => {
return (
<>
{0 && <Something/>}
{/* React: renders undesired 0 */}
{/* React Native: crashes 💥 */}

{'' && <Something/>}
{/* React: renders nothing */}
{/* React Native: crashes 💥 */}

{NaN && <Something/>}
{/* React: renders undesired NaN */}
{/* React Native: crashes 💥 */}
</>
)
}
```

This can be avoided by:
- casting the condition to bool: `{!!someValue && <Something />}`
- transforming the binary expression into a ternary expression which returns `null` for falsy values: `{someValue ? <Something /> : null}`

This rule is autofixable, check the Options section to read more about the different strategies available.

Examples of **incorrect** code for this rule:

```jsx
const Component = ({ count, title }) => {
return <div>{count && title}</div>
}
```

```jsx
const Component = ({ count }) => {
return <div>{count && <span>There are {count} results</span>}</div>
}
```

```jsx
const Component = ({ elements }) => {
return <div>{elements.length && <List elements={elements}/>}</div>
}
```

```jsx
const Component = ({ nestedCollection }) => {
return (
<div>
{nestedCollection.elements.length && <List elements={nestedCollection.elements} />}
</div>
)
}
```

```jsx
const Component = ({ elements }) => {
return <div>{elements[0] && <List elements={elements}/>}</div>
}
```

```jsx
const Component = ({ numberA, numberB }) => {
return <div>{(numberA || numberB) && <Results>{numberA+numberB}</Results>}</div>
}
```

```jsx
// If the condition is a boolean value, this rule will report the logical expression
// since it can't infer the type of the condition.
const Component = ({ someBool }) => {
return <div>{someBool && <Results>{numberA+numberB}</Results>}</div>
}
```

Examples of **correct** code for this rule:

```jsx
const Component = ({ elements }) => {
return <div>{elements}</div>
}
```

```jsx
// An OR condition it's considered valid since it's assumed as a way
// to render some fallback if the first value is falsy, not to render something conditionally.
const Component = ({ customTitle }) => {
return <div>{customTitle || defaultTitle}</div>
}
```

```jsx
const Component = ({ elements }) => {
return <div>There are {elements.length} elements</div>
}
```

```jsx
const Component = ({ elements, count }) => {
return <div>{!count && 'No results found'}</div>
}
```

```jsx
const Component = ({ elements }) => {
return <div>{!!elements.length && <List elements={elements}/>}</div>
}
```

```jsx
const Component = ({ elements }) => {
return <div>{Boolean(elements.length) && <List elements={elements}/>}</div>
}
```

```jsx
const Component = ({ elements }) => {
return <div>{elements.length > 0 && <List elements={elements}/>}</div>
}
```

```jsx
const Component = ({ elements }) => {
return <div>{elements.length ? <List elements={elements}/> : null}</div>
}
```

### Options

The supported options are:

### `validStrategies`
An array containing `"cast"`, `"ternary"` or both (default: `["ternary", "cast"]`) - Decide which strategies are considered valid to prevent leaked renders (at least 1 is required). The "cast" option will cast to boolean the condition of the JSX expression. The "ternary" option transforms the binary expression into a ternary expression returning `null` for falsy values. The first option from the array will be used as autofix, so the order of the values matter.

It can be set like:
```json5
{
// ...
"react/jsx-no-leaked-render": [<enabled>, { "validStrategies": ["ternary", "cast"] }]
// ...
}
```

Assuming the following options: `{ "validStrategies": ["ternary"] }`

Examples of **incorrect** code for this rule, with the above configuration:
```jsx
const Component = ({ count, title }) => {
return <div>{count && title}</div>
}
```

```jsx
const Component = ({ count, title }) => {
return <div>{!!count && title}</div>
}
```

Examples of **correct** code for this rule, with the above configuration:
```jsx
const Component = ({ count, title }) => {
return <div>{count ? title : null}</div>
}
```

Assuming the following options: `{ "validStrategies": ["cast"] }`

Examples of **incorrect** code for this rule, with the above configuration:
```jsx
const Component = ({ count, title }) => {
return <div>{count && title}</div>
}
```

```jsx
const Component = ({ count, title }) => {
return <div>{count ? title : null}</div>
}
```

Examples of **correct** code for this rule, with the above configuration:
```jsx
const Component = ({ count, title }) => {
return <div>{!!count && title}</div>
}
```

## When Not To Use It

If you are working in a typed-codebase which encourages you to always use boolean conditions, this rule can be disabled.

## Further Reading

- [React docs: Inline If with Logical && Operator](https://reactjs.org/docs/conditional-rendering.html#inline-if-with-logical--operator)
- [Good advice on JSX conditionals - Beware of zero](https://thoughtspile.github.io/2022/01/17/jsx-conditionals/)
- [Twitter: rendering falsy values in React and React Native](https://twitter.com/kadikraman/status/1507654900376875011?s=21&t=elEXXbHhzWthrgKaPRMjNg)
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -38,6 +38,7 @@ const allRules = {
'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'),
'jsx-no-constructed-context-values': require('./lib/rules/jsx-no-constructed-context-values'),
'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'),
'jsx-no-leaked-render': require('./lib/rules/jsx-no-leaked-render'),
'jsx-no-literals': require('./lib/rules/jsx-no-literals'),
'jsx-no-script-url': require('./lib/rules/jsx-no-script-url'),
'jsx-no-target-blank': require('./lib/rules/jsx-no-target-blank'),
Expand Down
137 changes: 137 additions & 0 deletions lib/rules/jsx-no-leaked-render.js
@@ -0,0 +1,137 @@
/**
* @fileoverview Prevent problematic leaked values from being rendered
* @author Mario Beltrán
*/

'use strict';

const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const isParenthesized = require('../util/ast').isParenthesized;

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const messages = {
noPotentialLeakedRender: 'Potential leaked value that might cause unintentionally rendered values or rendering crashes',
};

const CAST_STRATEGY = 'cast';
const TERNARY_STRATEGY = 'ternary';
const DEFAULT_VALID_STRATEGIES = [TERNARY_STRATEGY, CAST_STRATEGY];

/**
* @type {import('eslint').Rule.RuleModule}
*/
module.exports = {
meta: {
docs: {
description: 'Prevent problematic leaked values from being rendered',
category: 'Possible Errors',
recommended: false,
url: docsUrl('jsx-no-leaked-render'),
},

messages,

fixable: 'code',
schema: [
{
type: 'object',
properties: {
validStrategies: {
type: 'array',
items: {
enum: [
TERNARY_STRATEGY,
CAST_STRATEGY,
],
},
uniqueItems: true,
default: DEFAULT_VALID_STRATEGIES,
},
},
additionalProperties: false,
},
],
},

create(context) {
const config = context.options[0] || {};
const validStrategies = config.validStrategies || DEFAULT_VALID_STRATEGIES;
const fixStrategy = validStrategies[0];
const areBothStrategiesValid = validStrategies.length === 2;

function trimLeftNode(node) {
// Remove double unary expression (boolean cast), so we avoid trimming valid negations
if (node.type === 'UnaryExpression' && node.argument.type === 'UnaryExpression') {
return trimLeftNode(node.argument.argument);
}

return node;
}

function ruleFixer(fixer, reportedNode, leftNode, rightNode) {
const sourceCode = context.getSourceCode();
const rightSideText = sourceCode.getText(rightNode);

if (fixStrategy === CAST_STRATEGY) {
let leftSideText = sourceCode.getText(leftNode);
if (isParenthesized(context, leftNode)) {
leftSideText = `(${leftSideText})`;
}

const shouldPrefixDoubleNegation = leftNode.type !== 'UnaryExpression';

return fixer.replaceText(reportedNode, `${shouldPrefixDoubleNegation ? '!!' : ''}${leftSideText} && ${rightSideText}`);
}

if (fixStrategy === TERNARY_STRATEGY) {
let leftSideText = sourceCode.getText(trimLeftNode(leftNode));
if (isParenthesized(context, leftNode)) {
leftSideText = `(${leftSideText})`;
}
return fixer.replaceText(reportedNode, `${leftSideText} ? ${rightSideText} : null`);
}

throw new Error('Invalid value for "fixStrategy" option');
}

return {
'JSXExpressionContainer > LogicalExpression[operator="&&"]'(node) {
const leftSide = node.left;
const CAST_VALID_LEFT_SIDE_EXPRESSIONS = ['UnaryExpression', 'BinaryExpression', 'CallExpression'];
const isCastStrategyValid = areBothStrategiesValid || fixStrategy === CAST_STRATEGY;
const isCastValidLeftExpression = CAST_VALID_LEFT_SIDE_EXPRESSIONS.some(
(validExpression) => validExpression === leftSide.type
);

if (isCastStrategyValid && isCastValidLeftExpression) {
return;
}

report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', {
node,
fix(fixer) {
return ruleFixer(fixer, node, leftSide, node.right);
},
});
},

'JSXExpressionContainer > ConditionalExpression'(node) {
const isTernaryStrategyValid = areBothStrategiesValid || fixStrategy === TERNARY_STRATEGY;
if (isTernaryStrategyValid) {
return;
}

report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', {
node,
fix(fixer) {
return ruleFixer(fixer, node, node.test, node.consequent);
},
});
},
};
},
};

0 comments on commit 6b7b191

Please sign in to comment.