-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[No QA][TS migration] Adjust ESLint and TS configs #40778
Changes from 17 commits
51f2ed3
422e872
8d61837
df3e91c
bce33e4
b65414b
0748762
2d214d7
6e8a2be
c5a1f72
25b2935
af17bf5
b6f605d
57d2950
56395cd
76b1780
a06de12
85ff452
b47c28b
88a1e5b
d5083d2
a37b660
4c00bca
7e7be1d
9cdbda3
5f36384
84117ae
2cb2c5f
3468278
ccfd44b
5ebca88
7f6763c
6f2f703
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
**/node_modules/* | ||
**/dist/* | ||
android/**/build/* | ||
.github/actions/**/index.js" | ||
.* | ||
*.config.js | ||
**/node_modules/** | ||
**/dist/** | ||
android/**/build/** | ||
.github/actions/**/index.js | ||
docs/vendor/** | ||
docs/assets/** |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
// For all these Node.js scripts, we do not want to disable `console` statements | ||
module.exports = { | ||
rules: { | ||
// For all these Node.js scripts, we do not want to disable `console` statements | ||
'no-console': 'off', | ||
|
||
'@lwc/lwc/no-async-await': 'off', | ||
'no-await-in-loop': 'off', | ||
'no-restricted-syntax': ['error', 'ForInStatement', 'LabeledStatement', 'WithStatement'], | ||
}, | ||
}; |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious, why did these generated files change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea what caused this, I only changed eslint and TS configs. Any idea? 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it's because of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But let's make sure workflow tests are passing and watch for regressions 👀 |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
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.
question: does this line ignore anything that starts with
.
? If yes, great change! I was going to ask that we ignore.bundle
and.expo
too but it seems like that's covered.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.
Yes! I thought it was clever but turns out here are two caveats:
.github
and.storybook
weren't linted (it's more complicated because we want to lint.github
but not.github/actions/**/index.js
).*
, instead I did this: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.
@parasharrajat I checked it myself but could you double check that correct directories are linted (.github, .storybook, src, web, desktop etc) and correct are omitted (.github/actions/**/index.js, configs, dist, .expo, .git)? Thank you!
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.
Sure.
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 looks good to me.
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.
Some files are still showing errors via ts compiler though the EsLint is not triggered.