Thesis - eslint config #143
Conversation
ioay
force-pushed
the
eslint-config-thesis
branch
from
e02442e
to
25884ef
Compare
|
I let myself change the reference of the #99 task from "closes" to "refs". |
| // FIXME: | ||
| // This is temporary solution after changes of the eslint-config version: @thesis-co/eslint-config: "github:thesis/eslint-config#7b9bc8c" | ||
| // Overrides rules should be fixed file by file. | ||
| "overrides": [ | ||
| { | ||
| "files": [ | ||
| "src/components/Header/ConnectWallet.tsx", | ||
| "src/components/Modals/Support/MissingAccount.tsx", | ||
| "src/components/Modals/Staking/SignMessage.tsx", | ||
| "src/hooks/useDepositBTCTransaction.ts", | ||
| "src/components/shared/Form/FormTokenBalanceInput.tsx" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/no-misused-promises": "off" | ||
| } | ||
| }, | ||
| { | ||
| "files": [ | ||
| "src/hooks/useSignMessage.ts" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/no-floating-promises": "off" | ||
| } | ||
| }, | ||
| { | ||
| "files": [ | ||
| "src/theme/*" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/unbound-method": "off" | ||
| } | ||
| }, | ||
| { | ||
| "files": [ | ||
| "src/theme/Alert.ts" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/no-unsafe-member-access": "off" | ||
| } | ||
| }, | ||
| { | ||
| "files": [ | ||
| "src/components/shared/Form/FormTokenBalanceInput.tsx" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/no-unsafe-assignment": "off" | ||
| } | ||
| }, | ||
| { | ||
| "files": [ | ||
| "src/components/shared/TokenAmountForm/index.tsx" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/require-await": "off" | ||
| } | ||
| } | ||
| ], |
There was a problem hiding this comment.
Just one additional note here: in the future, it's probably better to do these directly in the file via /* eslint-ignore */ or // eslint-ignore-next comments. The core reasoning is that this leaves them in your face whenever you interact with those files, vs having them tucked away in an eslintrc that will likely be looked at once every 6 months at the most.
Glad we scoped this down a good bit regardless, thanks y'all!
There was a problem hiding this comment.
We want to revisit all of the ignores in a follow-up PR as soon as possible (hence the FIXME and referenced issue not closed yet #99).
I was afraid that leaving the comments inline the files may make it harder to distinguish which ignores are intentional and which we want to revisit and fix.
I agree 💯 inline disables should be preferred over the ones in eslintrc.
Thank you for chiming in sir! 🙇🏻
There was a problem hiding this comment.
Also I definitely meant eslint-disable*.
Even with inline disables I always encourage reasoning, so dropping a FIXME on it is def compatible. For example -> https://github.com/tahowallet/extension/blob/aa5c3b6ff3cee162314d473938d62aad41555b28/background/redux-slices/activities.ts#L1-L3, https://github.com/tahowallet/extension/blob/aa5c3b6ff3cee162314d473938d62aad41555b28/ui/components/Signing/Signer/SignerFrame.tsx#L9C1-L13 or https://github.com/tahowallet/extension/blob/aa5c3b6ff3cee162314d473938d62aad41555b28/ui/components/Shared/SharedIconRouterLink.tsx#L19-L20 (though the latter is arguably a bad example since we never completed the todo 😆).
Again though, this is really about future stuff, and I hear you re: the open issue. Nothing is ever perfect!
Refs: #99
Updated "@thesis-co/eslint-config" version to "github:thesis/eslint-config#7b9bc8.
Due to the above change, it was necessary to add a few eslint rules.
If we have time for it, we can also address changes to the related components to limit the number of added rules. The rule "@typescript-eslint/no-unsafe-member-access", applies to an external library that does not fit into our default eslint configuration, hence the need to leave it.