chore(fx-2514): enables jest eslint plugin #2858
Conversation
| @@ -15,7 +15,7 @@ import { | |||
| } from "lib/date" | |||
|
|
|||
| describe("date formatting", () => { | |||
| describe(timeRange, () => { | |||
| describe("timeRange", () => { | |||
There was a problem hiding this comment.
def think this form is a bit weird and unexpected and clever, but its also valid since it will read the function.name prop. Did this throw a warning / error?
There was a problem hiding this comment.
Title must be a string (eslintjest/valid-title)
There was a problem hiding this comment.
Non blocking, especially after all of this work was done, but i think that rule could be disabled. I can't remember where i read it but there's some interesting introspection benefits around passing the function in versus a string, but i also wonder about eslintjest setting this rule as a default -- in like a what do they know kind of way /shrug
There was a problem hiding this comment.
I'd be curious to know what the benefits are, if you ever dig it up. I'm inclined to roll with the string form since it's just more consistent and the default behavior of the recommended settings. There's a bit of discussion I dug up here jest-community/eslint-plugin-jest#431 for context as well.
dzucconi
force-pushed
the
dzucconi/chore/FX-2514--no-focus
branch
from
993d3a6
to
e0a24e9
Compare
Closes: FX-2514
OK, so I tend to use focused specs quite a bit when working, and frequently forget to unfocus them (and thanks to @damassi for always catching me). The jest plugin takes care of this with the recommended settings and will error.
We had eslint disabled over the specs — I think because of how common the
promise/always-returnrule was in violation. But, this is an easy automated refactoring: just use an async function. It's still disabled file-wide in a lot of places and I just left those alone. The rest of the errors I dealt with. Some much more valid/problematic. In general I think having eslint enabled over specs will help catch a lot of things.Right now the only things that are coming up as warns are disabled specs with skip. It's nice to have those highlighted in your editor as well.
I'd like to enable this in Force as well, unsure what will be involved with that.