Fix problems reported by eslint #179
Conversation
| @@ -63,13 +62,18 @@ const statusStyles = (props: StyleFunctionProps) => { | |||
| info: statusInfo, | |||
| } | |||
|
|
|||
| return styleMap[status] || {} | |||
| return styleMap[status as string] || {} | |||
There was a problem hiding this comment.
Non-blocking comment
The more I look at it the more I realize that this isn't how we should handle the different styles for the Alert component. 🤔 At that time, I used a solution from the threshold dashboard. But now I realize that we should handle it by passing colorScheme. Just as it was done in the Chakra UI.
However, let's leave it as it is but please add a TODO comment here or create a new issue that we should update this and start using a colorScheme instead of status props.
| const handleClick = () => { | ||
| requestAccount().catch((error) => { | ||
| throw error | ||
| }) | ||
| } |
There was a problem hiding this comment.
We duplicate this logic several times. Could we prepare a wrapper for these functions with a comment why we need this? Something very general to help us work with it.
function wrapAsyncFunction(func: Promise<void>) {
return () => {
func.catch((error) => {
throw error
})
}
}
There was a problem hiding this comment.
Yeah I think this is a good idea. But I would also consider disabling this rule,not sure about the consequnces, but it doesn't seem to be a very common pattern in React 🤔.
- https://github.com/orgs/react-hook-form/discussions/8622#discussioncomment-4060570
- https://typescript-eslint.io/rules/no-misused-promises/#checksvoidreturn
For example, if you don't mind that passing a () => Promise to a () => void parameter or JSX attribute can lead to a floating unhandled Promise:
There was a problem hiding this comment.
Let's add this wrapper instead of disabling this rule.
There was a problem hiding this comment.
@r-czajkowski To be honest, in the first turn, I felt that we should turn off this rule. I haven't seen such a pattern either. But then I found this issue. And I started to wonder if we shouldn't adapt our project to the new rule after all. It is still easy to do it at this phase.
But I also have some doubts about the consequences. Therefore, I have mixed feelings.
There was a problem hiding this comment.
I think we can leave the wrapper for now and revisit if it becomes really pain.
Fix problems reported by eslint but disabled in dapp/.eslintrc.
Closes: #99