[New Rule] jsx-embed-condition

[steelbrain]

This rule disallows use of && inside JSX Embeds to avoid conditional numbers from being rendered.

Fixes #1979

[ljharb]

I'm still not convinced this is a rule worth adding. If it were added, it'd need autofix to avoid being too disruptive.

[steelbrain]

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:

• "Text strings must be rendered within a Text component" when && operator is used in JSX facebook/react-native#20764
• Invariant Violation: Invariant Violation: Text strings must be rendered within a <Text> component. facebook/react-native#23607
• Text Strings must be rendered within a <Text> component facebook/react-native#23735
• Ensure edition_of is never an empty string artsy/emission#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!

[ljharb]

You can pass parserOptions.ecmaVersion directly in the one test case.

[steelbrain]

You can pass parserOptions.ecmaVersion directly in the one test case.

Attempted in 8abb5dc No dice. Any other ideas?

[ljharb]

I've rebased and hopefully fixed the tests. I'm not sold on the name of the rule - "embeds" isn't a term I've ever heard before to describe jsx curly-brace expressions.

Specifically, this is a rule concerned with preventing accidental rendering of a zero. This can happen only in the "children" position in jsx, or in a React.createElement call.

So:

• let's add support for React.createElement, both in the children position and in a literal children prop passed to that function
• let's rename the rule. perhaps, since it's no longer just jsx, no-conditional-children?

[ljharb]

Please mark the PR as ready for review once the next round of changes is done. Thanks!

[duncanbeevers]

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.

[duncanbeevers]

@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}`
            )
          });
        }
      }
    };
  }
};

[steelbrain]

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!

[Dakuan]

We've also been bit by this bug and would like to see this rule enforced in our code base

[wu-json]

Are these changes still active? Would love to have this rule

[ljharb]

@steelbrain are you still planning to finish this?

[holic]

@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.

[steelbrain]

I do not have the bandwidth in the near future and this needs more love than I can give right now :)
Good luck @holic

[ljharb]

Let's please keep this open so it can be reused.

[ljharb]

@holic please post a branch link here, and i'll update the PR

[holic]

Added a PR here: #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.

[ljharb]

@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.

[steelbrain]

Closed by #3203