search

ni / javascript-styleguide

JavaScript and TypeScript Style Guide
MIT License
9 stars 9 forks source link

Enable no-floating-promises for Angular tests #151

Open jattasNI opened 1 month ago

jattasNI commented 1 month ago

Describe the issue

The rule @typescript-eslint/no-floating-promises is currently enabled for most files but disabled for Angular tests. We should consider removing that configuration so it is enabled for all code.

Rationale:

  1. I recently found many instances of Angular tests that aren't handling promises returned from methods they call. (Here's a draft PR that enables this and some other async rules in systemlink-lib-angular). I can't point to a specific instance of problems this causes but it seems likely to cause intermittency.
  2. Our rationale for disabling it was that "The jasminewd2 library used by Angular applications results in a significant number of floating promises and unbound methods". I didn't see any instances of false positives like this in the PR above.

I recommend we try enabling it in a few places and evaluate the changes it requires. If we're happy with its suggestions we should enable it by default so that new projects get its benefits. If needed, apps can continue to disable it while they fix the issues it catches.

jattasNI commented 1 month ago

Consensus that this is a good direction. We should evaluate unbound-method too since it's disabled in the same place but that rule change could potentially happen in a separate release. Let's create a separate issue for that.

It would be good to combine this with #115