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
fix(linter): disabled eslint rules dont count as type checked rules #6897
fix(linter): disabled eslint rules dont count as type checked rules #6897
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/nrwl/nx-dev/13DfLewYp51kNqeCN7NQotmToQsA [Deployment for 166c03d canceled] |
Nx Cloud ReportCI ran the following commands for commit 166c03d. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch Sent with 💌 from NxCloud. |
I am not sure how https://nx.app/runs/SgMVzRnfgbe is failing as I didn't touch anything related to styled components. |
@kylecannon I also got hit by the nextjs failure, I believe I tracked it down to this: #6900 |
@JamesHenry good to know. I will rebase once it's merged. |
We actually think #6903 is more likely the fix for CI now |
1819aa9
to
166c03d
Compare
Thanks for the help @JamesHenry. My rebase passes 🥳 |
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.
Thank you for your contribution! LGTM 🎉
It doesn't work. I just create a new nx project without parserOptions in eslint. And it will throw errors like Parsing error: Cannot read file 'test\nx1290\tsconfig.json'. I think the migration shouldn't remove all parserOptions. It causes issue like this. When you create a new project
|
We are extending |
@JamesHenry im a little stumped what the root cause is ^ I basically re ran the old migration but only checked for 'off' |
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
#5798 migration sped up eslint for projects that don't need type checking. After spending several hours debugging, I found that the migration was not properly being ran for my workspace. It turned out if I remove the
parserOptions.project
in my.eslintrc.json
file, I would receive no linting errors and reduced the run time from over 3 minutes to under 15 seconds. Upon digging into my.eslintrc.json
file and looking at how the migration was done, oversight for overrides that turn off rules weren't accounted for and continued to keep the full type checking mode in the workspace project.Expected Behavior
The migration should not consider override rules that set the rule to
off
to be an enabled rule.To solve the issue on future NX releases, I created a migration that re-runs the previous 12.4.0 migration again with the fixes to save duplication of code and included an additional unit test for the edge case. If this is not acceptable please let me know the best way to go about having the next NX release re-run the previous migration for the next 12.9 release.