-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
REMOVE tslint, and use eslint for everything #6621
Conversation
This pull request is automatically deployed with Now. Latest deployment for this branch: https://monorepo-git-tech-remove-tslint.storybook.now.sh |
Codecov Report
@@ Coverage Diff @@
## next #6621 +/- ##
==========================================
+ Coverage 40.48% 40.57% +0.08%
==========================================
Files 637 637
Lines 8743 8714 -29
Branches 636 625 -11
==========================================
- Hits 3540 3536 -4
+ Misses 5106 5089 -17
+ Partials 97 89 -8
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## next #6621 +/- ##
==========================================
- Coverage 40.48% 40.46% -0.03%
==========================================
Files 637 637
Lines 8743 8754 +11
Branches 636 635 -1
==========================================
+ Hits 3540 3542 +2
- Misses 5106 5115 +9
Partials 97 97
Continue to review full report at Codecov.
|
3310029
to
d0d5ab3
Compare
# Conflicts: # yarn.lock
} | ||
} | ||
|
||
onToggle = (): void => { |
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.
What are these changes?
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.
Linting fixes, typescript files were missing a ton of useful eslint rules!
Now all eslint rules apply to typescript code as well 🎉
# Conflicts: # yarn.lock
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.
Love it! One weird configuration change above, otherwise looks fantastic 🎉
name: string; | ||
props: GenericProp; | ||
default?: boolean; | ||
}>; | ||
}[]; |
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.
Not sure about this one, I think Array<>
is easier to read. What other devs are thinking about it?
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 agree personally on that I find Array<X>
to be easier to read then X[]
, but this is a recommended rule from the typescript folk
what a strange world we live in where a triple slash means something other then a code comment
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.
Beautiful! 👌
@@ -94,13 +113,21 @@ module.exports = { | |||
error, | |||
{ allow: ['__STORYBOOK_CLIENT_API__', '__STORYBOOK_ADDONS_CHANNEL__'] }, | |||
], | |||
'@typescript-eslint/no-var-requires': ignore, | |||
'@typescript-eslint/camelcase': ignore, | |||
'@typescript-eslint/no-unused-vars': ignore, |
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.
Why?
presets: [ | ||
['@babel/preset-env', { shippedProposals: true, useBuiltIns: 'usage', corejs: 2 }], | ||
'@babel/preset-typescript', | ||
['babel-preset-minify', { builtIns: false, mangle: false }], |
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.
@ndelangen shouldn't we apply this one only in prod mode?
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.
yeah
Issue: TSlint will get replaced with ESlint at some point (see this article for explanations on why), having 2 linters is annoying:
What I did