eslint-plugin-react
eslint-plugin-react copied to clipboard
jsx-eslint
[New Rule] jsx-embed-condition
This rule disallows use of && inside JSX Embeds to avoid conditional numbers from being rendered.
Fixes https://github.com/yannickcr/eslint-plugin-react/issues/1979
I'm still not convinced this is a rule worth adding
It is a convention recommended by The official React docs. A whole bunch of people have been stung by this bug, many didn't even know what caused it, see:
- https://github.com/facebook/react-native/issues/20764
- https://github.com/facebook/react-native/issues/23607
- https://github.com/facebook/react-native/issues/23735
- https://github.com/artsy/emission/pull/1706
- https://blog.echobind.com/a-common-conditional-rendering-bug-in-react-native-8bbbc400abd8
(and you'll find many more examples by searching for "Text strings must be rendered within a Text component" on Google)
Apart from crashing React Native hard, the bug is known to add random 0s to the DOM in HTML renderer. I wrote this PR because my team got bit by this a few times.
If it were added, it'd need autofix to avoid being too disruptive.
This Lint rule is opt-in only. I can commit to adding an autofix for it later next month. My work schedule is pretty tight before that.
you can specify the parserOptions.ecmaVersion to be 2020 in this test case.
Doing so fixed the check on tests for latest Node.js runners, but broke them on the rest. I looked around the repository and couldn't find any tests with ecmaVersion newer than 2018. I would appreciate some help here! Thanks!
You can pass parserOptions.ecmaVersion directly in the one test case.
Attempted in https://github.com/yannickcr/eslint-plugin-react/pull/2888/commits/8abb5dcd900845d416f4469d7fc2fbf9197c4040 No dice. Any other ideas?
Please mark the PR as ready for review once the next round of changes is done. Thanks!
We have a similar rule in an internal codebase called logical-and-expressions; its existence in our jsx-ruleset makes it clear what its scope is.
Since this plugin's purview is a bit broader, perhaps a name like jsx-logical-and-expressions would capture the rule's scope+spirit.
@steelbrain Here's the internal rule we use, which includes a simple fixer.
module.exports = {
meta: {
docs: {
url: 'SECRET URL'
},
fixable: 'code'
},
create(context) {
const sourceCode = context.getSourceCode();
return {
JSXExpressionContainer(node) {
if (
node.expression.type === 'LogicalExpression' &&
node.expression.operator === '&&' &&
node.parent.type !== 'JSXAttribute'
) {
context.report({
node,
message: 'JSX expression should not use logical-and.',
fix: (fixer) => fixer.replaceText(
node,
`{${sourceCode.getText(node.expression.left)} ? ${sourceCode.getText(node.expression.right)} : null}`
)
});
}
}
};
}
};
I do not have the bandwidth to work on it this week, I'll continue this work on the weekend. Thank you for your continued support!
We've also been bit by this bug and would like to see this rule enforced in our code base
@ljharb Happy to open up a new PR with the lint rule + fix + tests if ya'll are interested in this. I was gonna release it as a separate package otherwise.
I do not have the bandwidth in the near future and this needs more love than I can give right now :) Good luck @holic
Added a PR here: https://github.com/jsx-eslint/eslint-plugin-react/pull/3318
This ports over the code we used internally at my last company that I was intending to release as its own package. Let me know if you'd prefer I build off of this PR instead.
@holic i thought i was pretty clear i did not want a second PR opened; that pollutes the repo forever. Now I have to keep both PRs open and in sync until both are landed together.