web-app icon indicating copy to clipboard operation
web-app copied to clipboard

Migrate from the deprecated TSLint to Angluar ESLint

Open rhopman opened this issue 1 year ago • 2 comments

Description

TSLint was deprecated by its creators in 2019. This PR replaces Codelyzer and TSLint with Angular ESLint, as recommended in the deprecation notice.

I followed the steps in Migrating from TSLint: Current Status as of angular-eslint v16 to migrate.

To resolve linting errors, I made the following code fixes:

  • To fix @angular-eslint/template/eqeqeq, I replaced == and != operators with === and !== respectively, after verifying each occurrence to make sure that there would be no functional impact.
  • To fix @angular-eslint/no-empty-lifecycle-method (Lifecycle methods should not be empty), I removed all empty lifecycle methods (mostly ngOnInit).
  • To fix @angular-eslint/no-output-native (Output bindings, including aliases, should not be named as standard DOM events), I renamed @Output() submit to @Output() submitEvent.
  • To fix @angular-eslint/template/no-negated-async (Async pipe results should not be negated), I changed !(isHandset$ | async) to (isHandset$ | async) === false in shell.component.html.

After the above fixes, npx ng lint runs without issues.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • [x] If you have multiple commits please combine them into one commit by squashing them.

  • [x] Read and understood the contribution guidelines at web-app/.github/CONTRIBUTING.md.

rhopman avatar Sep 26 '24 08:09 rhopman

This is my first PR on the project so I hope I did everything right. Is there anybody who could review this, and let me know if there's anything I need to do or change? Perhaps @alberto-art3ch?

rhopman avatar Oct 08 '24 09:10 rhopman

@PC-11-00 @alberto-art3ch @janez89 @somasorosdpc is anybody able to review this PR please? I realize it touches a lot of files but the changes are all pretty minor. If somebody has a better approach to get rid of this deprecated dependency I'm open to suggestions!

rhopman avatar Oct 10 '24 14:10 rhopman

@rhopman I am sorry to be slow on this PR. Can you please resolve the conflicts and we can give a try to review it

adamsaghy avatar Nov 06 '24 09:11 adamsaghy

Thanks, @adamsaghy! I've resolved the merge conflicts. Please let me know if there's anything else I can do to assist in the process.

rhopman avatar Nov 06 '24 10:11 rhopman

@alberto-art3ch Please help me to review this PR.

adamsaghy avatar Nov 06 '24 12:11 adamsaghy

@rhopman Please kindly check the failing lint validation:

Linting "mifosx-web-app"...

/home/runner/work/web-app/web-app/src/app/loans/loans-view/account-details/account-details.component.html
Error:   14:26   error  Expected `===` but received `==`  @angular-eslint/template/eqeqeq
Lint errors found in the listed files.

Error:   14:73   error  Expected `!==` but received `!=`  @angular-eslint/template/eqeqeq
Error:   14:126  error  Expected `!==` but received `!=`  @angular-eslint/template/eqeqeq

✖ 3 problems (3 errors, 0 warnings)


Linting "mifosx-web-app-e2e"...

All files pass linting.

Error: Process completed with exit code 1.

adamsaghy avatar Nov 06 '24 12:11 adamsaghy

Apologies @adamsaghy, the linting issue should be fixed now.

rhopman avatar Nov 06 '24 14:11 rhopman