chore(eslint-config): update typescript-eslint to latest #7010
Conversation
| '@typescript-eslint/prefer-promise-reject-errors': 'off', | ||
| '@typescript-eslint/only-throw-error': 'off', |
There was a problem hiding this comment.
These new recommended ones I'm actually not sure if they are helpful to be honest, so I'm planning to keep those disabled, tell me if you have any concerns with that
There was a problem hiding this comment.
Both of these appear to me to be helpful for ensuring our error formatting is consistent.
https://typescript-eslint.io/rules/only-throw-error/
https://typescript-eslint.io/rules/prefer-promise-reject-errors/
I think I'd be in favor of having them on if its not a big lift. Also something we could do later. Are there situations where they would be not helpful I might not be thinking of? (Not a blocker, changes look good!)
There was a problem hiding this comment.
Talked with Le Roux also and I'll take another pass on those, but I also don't think the rule really works as well as the description shows. To give you an example, every re-thrown error triggers this rule and we have to manually assert Error on it (because we never know the type), this just seems unhelpful and we re-throw quite often due to logging. But just to re-iterate, I'll take a closer look there when following up just to make sure I'm not disabling something actually useful, thanks for sharing your thoughts on that!
gribnoysup
added
the
no-title-validation
| definitions are pretty strict. We require these disabled to avoid | ||
| tests flaking out hence ignoring the usage here. | ||
| @ts-ignore */} | ||
| @ts-expect-error ^^^ */} |
There was a problem hiding this comment.
That was the easiest way to deal with no-ts-ignore... 😅
I'm slowly continuing to update eslint to latest, this patch updates typescript-eslint to the latest version, this actually brings a lot of changes to the recommended rules, to avoid making this totally unreviewable I'm currently switching all the new or updated rules to
warnand will go through them one by one in the follow-ups.Most of those are actually not encountered that often or autofixable / easy to fix, so it should be a pretty straightforward follow-up process.
The only one that's real nasty is the
no explicit anyone. Even though it was on by default before and we even have explicit disabling where appropriate, something changed in how the rule is detected or applied and so there's a ton of legit cases where it gets triggered in the code (and it's hard for me to tell why this wasn't happening before)