feat(eslint-plugin): [use-lifecycle-interface] add fixer for the rule #1691
Conversation
bulldog98
changed the title
packages/eslint-plugin/tests/rules/use-lifecycle-interface/cases.ts
Outdated
Show resolved
Hide resolved
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 87c13f5. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targetsSent with 💌 from NxCloud. |
| @@ -120,6 +138,19 @@ export const invalid = [ | |||
| }, | |||
| }, | |||
| ], | |||
| annotatedOutput: ` | |||
| @Injectable() | |||
| class Test implements DoBootstrap { | |||
There was a problem hiding this comment.
This didn't fix all the issues?
There was a problem hiding this comment.
If I understood the documentation of eslint correctly the fixes are applied again and again, as long as the lint error is still reported. I'm unsure, how I can use the plugin in a local testing repo to test that out, please advice me on that.
There was a problem hiding this comment.
So yes I found a way to try it out. (run e2e tests and then try your changes in tmp/e2e-fixtures/ in any of the apps there. Maybe that should be documented) Indeed eslint runs again on the output of fixes until there are no changes, so it indeed results in a fix that adds all the implements clauses.
There was a problem hiding this comment.
Only thing I noticed is that I do not add an import for the Interface, here I would like guidance @JamesHenry
There was a problem hiding this comment.
@bulldog98 Sorry for the delay. We have a utility for it used in a few other rules, it's very straightforward: #1829
There was a problem hiding this comment.
I want to get things ready for Angular 18, so I went ahead and applied the change
|
Thank you @bulldog98! Left some comments |
|
I'll try to fix the issues, not sure if I have time before the weekend though |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1691 +/- ##
=======================================
Coverage 90.46% 90.46%
=======================================
Files 178 178
Lines 3345 3347 +2
Branches 536 536
=======================================
+ Hits 3026 3028 +2
Misses 185 185
Partials 134 134
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
I don't seem to be able to push to the PR, I have added the import fix in a separate commit |
|
I think it's because you opened the PR from your main branch maybe? |
|
Thanks again @bulldog98! |
|
Thanks for adding the import part @JamesHenry, I'll try to keep in mind to open from a separate branch in the future. |
I got the fixer to work, but could not fix one of the tests, seems to be a white space issue.