contiamo / operational-scripts

A TypeScript-first toolchain for developing front-end applications.
MIT License
7 stars 2 forks source link

tslint rules: allow fast null checks #21

Closed micha-f closed 6 years ago

micha-f commented 6 years ago

Is your feature request related to a problem? Please describe. I'd like to use logical operators to perform fast null checks and perform method or function calls for side effects (e.g. e && e.preventDefault()).

Currently this results in tslint complaining about unused expression, expected an assignment or function call.

Describe the solution you'd like Modify the tslint rule to be "no-unused-expression": [true, "allow-fast-null-checks"]

Describe alternatives you've considered Not using short circuits for fast null checks. But I quite like them.

Additional context https://palantir.github.io/tslint/rules/no-unused-expression/

TejasQ commented 6 years ago

The reasoning behind this lint setting is that expressions really should have a reason for existing outside of being conditionals in order to more clearly describe intent to developers, and to prevent associated pitfalls with performing conditional checks with such expressions.

Consider the case where "oh, I'll just use an expression here and it'll be fine", yet the expression resolves to a false and the use case is a string interpolation.

<MyComponent className={`${thisIsFalseSometimes && "my-class"} my-other-class`}

This would unnecessarily give you DOM with <div class="false my-other-class, which is unexpected and could lead to unpredictable behavior of an application. This is, in essence, why this rule is necessary.

One possible solution to your use case would be to simply be more explicit about intent:

if (e) {
  e.preventDefault();
}

would do the same thing, while being a little more intelligible to future readers.

Does that solve your problem? Is there a really strong argument/use case for this change to the lint settings? I'd love to hear more if so.

micha-f commented 6 years ago

In the example you made, the expression is actually used as an expression. The example you made does not violate this tslint rule.

micha-f commented 6 years ago

I'm ok with leaving the rule as it is. I'm also completely fine with if (e) e.something() - which is what I had.