-
Notifications
You must be signed in to change notification settings - Fork 3
fix: Support ESLint 8.x #136
Conversation
@@ -9,6 +9,7 @@ module.exports = { | |||
{ | |||
files: ['**/*.js', '**/*.jsx'], | |||
rules: { | |||
'@typescript-eslint/no-unsafe-argument': 'off', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@typescript-eslint/no-unsafe-argument is a new recommended rule that is likely to result in a bunch of linting errors across existing projects. For reference, skuba itself required the following tweaks: seek-oss/skuba@b887ccf
Currently, we're only disabling it for JavaScript source files and tests. Should we be less aggressive across the board by e.g. defaulting it to warn
instead of error
to begin with? The counterpoint is that warnings are often left ignored, which is why we tend to avoid warn
s elsewhere in the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if you have an opinion here @samchungy, I noticed the Orders API has some warnings 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like a good rule but I can see how it would be noisy on an existing code base. I'm leaning towards leaving it on and trusting upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hahaha don't look at that repo... 🙈 Funnily enough it's not even the most pressing issue with that repo 😭.
My plan was to leave those warnings on until it annoys enough people for us to allocate time to it 😂. Leave it on is my opinion too. Worst case they can turn it off
Dogfooding seek-oss/eslint-config-seek#73.