Investigate turning on no-unused-expressions



2 years ago
7 months ago


(Reporter: standard8, Unassigned)


3 Branch
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


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
Depends on: 1409694
Product: Testing → Firefox Build System
Depends on: 1502309
Depends on: 1502312
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.