Regression in v8.2.4+ / v9.0.0.beta: optional chaining without assignments trigger no-unused-expressions

[lehni]

Unfortunately, #595 didn't solve all issues with optional chaining for me, or new ones got introduced in the meantime. While some uses of optional chaining seem to work now, others are causing new linting errors with v8.2.4 and v8.2.5:

error  Expected an assignment or function call and instead saw an expression  no-unused-expressions

The statement in question is:

this.element?.destroy()

v8.2.1 is still the last version that I am able to use for optional chaining

[lehni]

The same problem also occurs in 9.0.0-beta.2

[lehni]

This is also still happening in v8.2.6 / v9.0.0-beta.3

[lehni]

@existentialism may I point out this issue here that's still happening with optional chaining? I believe this only happens when optional chaining is used outside of an assignment. As soon as I turn the above statement into an assignment, optional chaining works:

const res = this.element?.destroy()

[macrozone]

even with assigments i get this error:

[eslint] 'current' is not defined. (no-undef)

using version 9.0.0

[lehni]

I guess assignments without = do this?

[lehni]

It's a pitty that this made it into 9.0.0 without a fix... @rubennorte since you knew how to fix #595 / #630, perhaps you could take a look at this issue?

[kennethnym]

no-unused-expressions is also triggered when chaining promises with .then?.().

[kennethnym]

Here is a test snippet:

The error:

[kennethnym]

Sorry for the syntax error, but the error appears in either case.

[kaicataldo]

The fix for this is most likely in the forked version of no-unused-expressions in eslint-plugin-babel.

Edit: Typo in link

[kennethnym]

Where is the forked version? The link doesn't seem to contain it.

[remmycat]

Not sure what you mean with forked version @kaicataldo. The link href is broken, but the actual eslint-plugin-babel repo (link text) also does not seem to use a forked version of babel-eslint.
It actually uses 8.2.2 on master, which was of course before the regression happened.

Edit: Ah, i misread what part was forked, sorry!

[lehni]

I believe it's this file here: https://github.com/babel/eslint-plugin-babel/blob/master/rules/no-unused-expressions.js

[kaicataldo]

Sorry for the typo in the link. @lehni has the correct one!

[lehni]

@kaicataldo but what's this file forked from?

[lehni]

Looking at this more closely, it looks like it's not so much forking code. It's extending the rules contained in this:

https://github.com/eslint/eslint/blob/master/lib/rules/no-unused-expressions.js

[lehni]

Ok, I think I've figured this out now:

1. By default babel-eslint doesn't support such language additions. There is the plugin eslint-plugin-babel, which provides rules that replace the default ones, but those aren't loaded by default. You have to add the plugin as outlined here, and then you need to replace the default rules with these babel/* versions.

2. When not doing so, the default rules from eslint are used, as linked to above, which definitely do not support OptionalCallExpression.

3. Even when doing so, this currently still fails.

4. Solution: Support optional calls in no-unused-expressions eslint-plugin-babel#158

Fingers crossed for this getting merged soon.

[lehni]

If you can't wait for the release, you can already use it now:

package.json

  "devDependencies": {
    ...
    "eslint-plugin-babel": "github:lehni/eslint-plugin-babel#b89ec98f1bc3e249ff39bb6094c9654b6e02dbaf",
    ...
  }

.eslintrc.js

module.exports = {
  ...
  plugins: [
    'babel',
    ...
  ],
  ...
  rules: {
    ...
    // https://github.com/babel/eslint-plugin-babel/pull/158
    'no-unused-expressions': 'off',
    'babel/no-unused-expressions': 'error',
    ...
  }
}

[lehni]

Update: eslint-plugin-babel version 5.2.0 is out now, with the above fix merged in. 🎉