Gm/update-base-and-readme #32
Conversation
| @@ -34,6 +34,32 @@ export default tsEslint.config( | |||
| "@typescript-eslint/consistent-type-exports": "error", | |||
| "@typescript-eslint/no-import-type-side-effects": "error", | |||
| "@typescript-eslint/consistent-type-imports": "error", | |||
| "@typescript-eslint/use-unknown-in-catch-callback-variable": "off", | |||
There was a problem hiding this comment.
Things I think are true:
- errors should be
unknown - your tsconfig settings should be strict
Things I want to be true:
- that ts setting works in
then().catch()(it does not yet) - I don't have to fix it myself (apparently you can override the global, but i don't want to do that.
Therefore, I don't think this rule is important enough (and i don't feel like making that many code changes to support it 😬 )
There was a problem hiding this comment.
I can't say I've encountered this but your answer seems reasonable to me.
There was a problem hiding this comment.
TS made the unknown thing a default a bit ago, and I got wrecked with a refactor of tons of tiny changes from any to unknown. But apparently, that was only for try/catch, not then().catch()
So we wait on...
| "@typescript-eslint/restrict-template-expressions": [ | ||
| "error", | ||
| { | ||
| allowAny: true, | ||
| allowBoolean: true, | ||
| allowNullish: false, | ||
| allowNumber: true, | ||
| allowRegExp: true, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
This rule I catches some nice little bugz that happen inside template literals, like [object] [object] and encourages you to provide fallbacks for nullish values.
Template literals will call the .toString() method on everything you put there, so with that understanding, I think it is fine to allow the above values, as toString() was not adding any value having that enforced.
However, it did make me consider numbers, as most numbers can be handled fine by toString(), big numbers in UI should be displayed with .toLocaleString(), and any floating point math should be dealt with before you end up with ugly 3.0000000000000001.
I kept the allowNullish: false however. The more time that goes by, the more I'm bothered by my logs that end up with: "error with item id:" where the item didn't exist or something. This rule would ask the developer to provide some feedback, like
`error with item id: ${item?.id ?? "<item not found>"}`https://typescript-eslint.io/rules/restrict-template-expressions/
| "@typescript-eslint/no-confusing-void-expression": [ | ||
| "error", | ||
| { | ||
| ignoreArrowShorthand: true, | ||
| ignoreVoidOperator: true, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Ok, I started my journey here without the overrides. And then like 50 files in... it started to feel a bit overboard...
The core of this rule supports lots of good patterns, but for these 2 overrides, i feel like I'm comfortable not going all the way.
This means, we can still:
<button onClick={() => console.log("🎉")} />https://typescript-eslint.io/rules/no-confusing-void-expression/
| "@typescript-eslint/no-unused-expressions": [ | ||
| "error", | ||
| { | ||
| allowShortCircuit: true, | ||
| allowTernary: true, | ||
| enforceForJSX: true, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Ok, this one was a bit funky. Essentially it is trying to protect you from misunderstanding function calls, but it went a little far. They provided the above overrides, which maintained our current coding style, while adding som protection when you actually do something wrong.
https://eslint.org/docs/latest/rules/no-unused-expressions#options
I have applied this to all our ts.
I found a few errors in the readme example, and I added some new rules to the base config that I found helpful in all the other repos