-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
6 changed files
with
1,028 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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: | ||
- coercing the conditional to a boolean: `{!!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 `"coerce"`, `"ternary"`, or both (default: `["ternary", "coerce"]`) - Decide which strategies are considered valid to prevent leaked renders (at least 1 is required). The "coerce" option will transform the conditional of the JSX expression to a boolean. The "ternary" option transforms the binary expression into a ternary expression returning `null` for falsy values. The first option from the array will be the strategy used when autofixing, so the order of the values matters. | ||
|
||
It can be set like: | ||
```json5 | ||
{ | ||
// ... | ||
"react/jsx-no-leaked-render": [<enabled>, { "validStrategies": ["ternary", "coerce"] }] | ||
// ... | ||
} | ||
``` | ||
|
||
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": ["coerce"] }` | ||
|
||
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) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
/** | ||
* @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 COERCE_STRATEGY = 'coerce'; | ||
const TERNARY_STRATEGY = 'ternary'; | ||
const DEFAULT_VALID_STRATEGIES = [TERNARY_STRATEGY, COERCE_STRATEGY]; | ||
const COERCE_VALID_LEFT_SIDE_EXPRESSIONS = ['UnaryExpression', 'BinaryExpression', 'CallExpression']; | ||
|
||
function trimLeftNode(node) { | ||
// Remove double unary expression (boolean coercion), so we avoid trimming valid negations | ||
if (node.type === 'UnaryExpression' && node.argument.type === 'UnaryExpression') { | ||
return trimLeftNode(node.argument.argument); | ||
} | ||
|
||
return node; | ||
} | ||
|
||
function ruleFixer(context, fixStrategy, fixer, reportedNode, leftNode, rightNode) { | ||
const sourceCode = context.getSourceCode(); | ||
const rightSideText = sourceCode.getText(rightNode); | ||
|
||
if (fixStrategy === COERCE_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 TypeError('Invalid value for "validStrategies" option'); | ||
} | ||
|
||
/** | ||
* @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, | ||
COERCE_STRATEGY, | ||
], | ||
}, | ||
uniqueItems: true, | ||
default: DEFAULT_VALID_STRATEGIES, | ||
}, | ||
}, | ||
additionalProperties: false, | ||
}, | ||
], | ||
}, | ||
|
||
create(context) { | ||
const config = context.options[0] || {}; | ||
const validStrategies = new Set(config.validStrategies || DEFAULT_VALID_STRATEGIES); | ||
const fixStrategy = Array.from(validStrategies)[0]; | ||
|
||
return { | ||
'JSXExpressionContainer > LogicalExpression[operator="&&"]'(node) { | ||
const leftSide = node.left; | ||
|
||
if ( | ||
validStrategies.has(COERCE_STRATEGY) | ||
&& COERCE_VALID_LEFT_SIDE_EXPRESSIONS.some((validExpression) => validExpression === leftSide.type) | ||
) { | ||
return; | ||
} | ||
|
||
report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', { | ||
node, | ||
fix(fixer) { | ||
return ruleFixer(context, fixStrategy, fixer, node, leftSide, node.right); | ||
}, | ||
}); | ||
}, | ||
|
||
'JSXExpressionContainer > ConditionalExpression'(node) { | ||
if (validStrategies.has(TERNARY_STRATEGY)) { | ||
return; | ||
} | ||
|
||
report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', { | ||
node, | ||
fix(fixer) { | ||
return ruleFixer(context, fixStrategy, fixer, node, node.test, node.consequent); | ||
}, | ||
}); | ||
}, | ||
}; | ||
}, | ||
}; |
Oops, something went wrong.