chore: Stricter ESLint config #383
Conversation
going-confetti
requested review from
e-fisher and
Llandy3d
and removed request for
a team
.eslintrc.cjs
Outdated
| '@typescript-eslint/no-floating-promises': 'off', | ||
| '@typescript-eslint/no-misused-promises': 'off', |
There was a problem hiding this comment.
I suggest turning those on later. It highlighted some cases where we pass async functions as e.g. click handlers
There was a problem hiding this comment.
Maybe add them as warnings?
There was a problem hiding this comment.
Initially, I was reluctant to do this, as the number of warning seemed to be too high with quite a few of them not being useful. I then discussed possible solutions with @e-fisher and we found a way to enable these rules with a few tweaks.
| @@ -24,6 +24,8 @@ export function stringify(value: unknown): string { | |||
| return `{${properties}}` | |||
| } | |||
|
|
|||
| // TODO: https://github.com/grafana/k6-studio/issues/277 | |||
There was a problem hiding this comment.
Marked ignore comments with a link to the issue (I'll update the issue description once this PR is merged).
While it does look annoying, the pros of enabling stricter rules for everything else outweigh the cons
| // TODO(router): useLocation is not type-safe. Refactor this route to avoid using it. | ||
| const { state } = useLocation() as { state: { externalScriptPath: string } } |
There was a problem hiding this comment.
@allansson now I see what you meant, and I barely scratched the surface...
There was a problem hiding this comment.
The same goes for getting route params via useParams or search params via useSearchParams. You're just casting things hoping that your routes are structured in a certain way.
.eslintrc.cjs
Outdated
| '@typescript-eslint/no-floating-promises': 'off', | ||
| '@typescript-eslint/no-misused-promises': 'off', |
There was a problem hiding this comment.
Maybe add them as warnings?
Relevant issue: #277