Migrate eg2 from TSLint to ESLint

Bug #1959048 reported by Jane Sandberg
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

After the upgrade to Angular 12 (see bug 1948035), running `ng lint` prints the following message:

TSLint's support is discontinued and we're deprecating its support in Angular CLI.
To opt-in using the community driven ESLint builder, see: https://github.com/angular-eslint/angular-eslint#migrating-an-angular-cli-project-from-codelyzer-and-tslint.

The linting is awesome and keeps us consistent -- we should follow the recommended steps to make sure we can continue using a supported linter.

Revision history for this message
Jane Sandberg (sandbergja) wrote :

I ran through the generator steps here, as well as `ng lint --fix` to apply some fixes: user/sandbergja/lp1959048_eslint

To try it out on an existing installation, you'll just want to run npm install to install the dependencies.

It is a bit pickier than the old linter, for better or worse. It does look for objectionable bits in the HTML templates as well as the ts files, which seems like a nice improvement. However, there are 121 errors that the new linter picked up that it couldn't fix automatically. :-(

Revision history for this message
Galen Charlton (gmc) wrote :

I took a look at this today and reviewed the classes of errors that ng lint --fix couldn't deal with.

These two classes look straightforward to deal with, just a little time-consuming:

     84 @angular-eslint/no-empty-lifecycle-method
    130 @angular-eslint/template/eqeqeq

However, I think these need a little thought:

     44 @angular-eslint/no-output-on-prefix

This is coming from the Angular style guide, specifically https://angular.io/guide/styleguide#dont-prefix-output-properties, which doesn't mention any particular risk of bugs by having output properties named on*.

For the sake of both time and reducing rework of pending pull requests that use core eg components, I suggest that

* this check be turned off where present for components under src/app/share, then slated for cleanup on a component-by-component basis while avoiding forcing much churn on pending pull requests
* fixed for components elsewhere, as those typically will have only one or two consumers each

I do think that we should not add any further on* output properties, so I'm not advocating for turning off this check for the entire project.

Changed in evergreen:
assignee: nobody → Jane Sandberg (sandbergja)
milestone: none → 3.10-beta
Revision history for this message
Jane Sandberg (sandbergja) wrote :

I've rebased my branch and done the manual lint fixes per Galen's suggestions: user/sandbergja/lp1959048_eslint

tags: added: pullrequest
Changed in evergreen:
assignee: Jane Sandberg (sandbergja) → nobody
Galen Charlton (gmc)
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
importance: Undecided → Wishlist
status: New → Confirmed
Revision history for this message
Galen Charlton (gmc) wrote :

Tested! I've pushed a signoff branch to user/gmcharlt/lp1959048_signoff; this branch also includes a release notes entry.

tags: added: signedoff
Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
Revision history for this message
Jane Sandberg (sandbergja) wrote :

Thanks, Galen! I confirmed with Bill that this won't interfere with the patron angular work. Pushed to master.

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.