Investigate turning on no-unused-expressions

NEW
Unassigned

Status

enhancement
P2
normal
2 years ago
4 months ago

People

(Reporter: standard8, Unassigned)

Tracking

3 Branch
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 years ago
no-unused-expressions can catch a few issues in the code, that generally are bugs or could lead to strict warnings.

Although we do invoke various properties for side-effects intentionally, I think it is probably worth turning this on anyway, as it will highlight those.

As an initial stab, it seems we'll want this in recommended.js:

    "no-unused-expressions": ["error", {
      "allowShortCircuit": true,
      "allowTernary": true
    }],
(Reporter)

Updated

2 years ago
Depends on: 1409694

Updated

a year ago
Product: Testing → Firefox Build System
(Reporter)

Updated

6 months ago
Depends on: 1502309
(Reporter)

Updated

6 months ago
Depends on: 1502312
(Reporter)

Comment 1

6 months ago
I took another look at this today. First an update config:

+    "no-unused-expressions": ["error", {
+      "allowShortCircuit": true,
+      "allowTaggedTemplates": true,
+      "allowTernary": true,
+    }],

With this, there's about 200 errors. The majority of these are false positives. There's a few actual issues which I'll file as blockers as I get time, though the more worrying bugs are already filed.

Of the false positives, there's a lot of places where we do `element.clientTop` or similar to force a layout flush or hooking up of xbl. For those we probably want to think about if we could have a utility function, e.g. forceFlush(element), so that this is explicit in the code, and then ESLint doesn't warn about it.

For other intentional things we could just do // eslint-disable-next-line no-unused-expressions or similar.

The few things we have caught make me think it is worth moving on this at some stage.
Priority: P3 → P2
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.