chore: lint all the things #131
Conversation
wejendorp
changed the title
There was a problem hiding this comment.
Really nice that you improved the code of this repo to follow our lining rules a bit more 💪
I am a bit worried about the changes related to the react hooks. Adding things to the dependency arrays will potentially change how the code behaves. Thorough testing is required on this PR!
Also I don't think we should disable the rules-of-hooks as they are quite important to follow (unless we are absolutely sure we want to ignore them a few places). I think it can be hard to reason about code that doesn't follow those rules, and to maintain it.
Lastly, can we change to use @kapeta/eslint-config? I think some rules in this repo are stricter than what we need.
| @@ -21,7 +21,7 @@ export const kapetaDark = createTheme( | |||
| { | |||
| components: { | |||
| MuiLink: { | |||
| //@ts-ignore | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
Maybe we can make a habit of explaining in the comment why we tell TS to ignore an error whenever we use @ts-ignore so we know the reason for it.
| // TODO: This seems bad | ||
| /* eslint-disable react-hooks/rules-of-hooks */ |
| @@ -13,12 +12,13 @@ interface Props { | |||
|
|
|||
| export const SamplePlanSection = (props: Props) => { | |||
| const title = props.sample.content.metadata.title ?? props.sample.content.metadata.name; | |||
| const hack = useLoadedPlanContext(props.sample.content); | |||
There was a problem hiding this comment.
Not really any more, I started out throwing it around a bit and it seems like it ended up working :)
src/renderer/utils/taskGetter.ts
Outdated
| // TODO: seems bad | ||
| /* eslint-disable react-hooks/rules-of-hooks */ |
There was a problem hiding this comment.
Yes, why disable it? IMO the hook rules are quite important to avoid hard-to-debug bugs
There was a problem hiding this comment.
I'll change them to warnings, then we still get the squiggles, but it won't block the build.
Fix linter issues in almost all places.
This should make the linter pass, so we can start caring about it again.