Eslint-plugin-react: Rule proposal: Disallow unused JSX expressions

[Daniel15]

There's probably a better name for this, I just couldn't think of it :stuck_out_tongue:

These are valid:

function foo() {
  return (
    <div>Hello world!</div>
  );
}

var x = <div>Hello world!</div>;

These are invalid:

function foo() {
  <div>Hello world!</div>;
}

if (something) {
  <div>Hello world!</div>;
}

While doing a code review, I encountered some code that missed a return statement and just had the JSX sitting there. No lint error :(

[ljharb]

+1, this should parallel http://eslint.org/docs/rules/no-unused-expressions and probably be called no-unused-jsx-expressions

[Daniel15]

Yeah, that name is much nicer :+1:

[Daniel15]

Looks like this was implemented as require-render-return, at least for the simple cases.

[ljharb]

I still think it would be useful on its own.

[Daniel15]

I hit something similar again today:

      {sections.map(section => {
        <AdsHelpTrayContentList
          contentList={section.items}
          key={section.title}
          title={section.title}
          onItemSelected={props.onItemSelected}
        />
      })}

Can you see the problem?

...

I used { } instead of ( ) in the arrow function for the map call, so the function wasn't actually returning anything. 😠

[golopot]

Since eslint core rule no-unused-expressions has covered jsx expressions now, is this proposal still relevant?

[ljharb]

@golopot when was that added?

[golopot]

@ljharb demo .

[ljharb]

I see that it works, but since https://github.com/eslint/eslint/pull/2030 eslint is explicitly supposed to not care about variable usage inside jsx, so I’m wondering when it started checking unused jsx expressions.

[golopot]

I think that the commit you referred to only affects no-unused. The implementation of no-unused-expressions does not rely on .eslintUsed property.

[ljharb]

ok, so the implication is that this capability has always existed in eslint and this rule wasn’t needed.

Closing, but will reopen if @Daniel15 confirms there’s still a gap.

[Daniel15]

Thanks! I'll look into no-unused-expressions. Look like at Facebook we had originally disabled that rule because it showed a warning for foo && foo.bar(), but it looks like the allowShortCircuit config option can be set to allow that. I'll try it out.