chore: migrate tslint to eslint #199

the-ult · 2021-04-13T18:47:32Z

What

Migrate from TSLint to ESLint
and upgrade dependencies

Why

To provide the 'best' examples possible, we should make sure the examples are making use of the best practices. Like discussed here
To help doing that, there are 2 great ESLint plugins:

To be able to use these plugins we first need to migrate from TSLint to ESLint.

This PR is a prerequisite of #198 (Adding the above plugins and improve the examples)

How

Upgrade to NX 12.0.3
- Needed to be able to use the @nrwl/angular:convert-tslint-to-eslint schematic
Move apps/example-app/app/examples to apps/example-app/app/examples
- Needed to be able to use the @nrwl/angular:convert-tslint-to-eslint schematic since the schematic expects the 'NX' structure for an app
Migrate TSLint to ESLint
- example-app
- jest-utils
- testing-library
ESLint
- Remove duplicate rules which are already provided by NX or other [plugin]/recommended

Troubleshooting

Removing TSLint
Could not remove tslint from the package.json due to the following errors in a migration file:

projects/testing-library/migrations/4_0_0/rules/noComponentParametersRule.ts
(2,49): error TS2307: Cannot find module 'tslint' or its corresponding type declarations.
projects/testing-library/migrations/4_0_0/rules/noComponentParametersRule.ts
(38,82): error TS2339: Property 'ruleName' does not exist on type 'Rule'.
projects/testing-library/migrations/4_0_0/rules/noComponentParametersRule.ts
(39,66): error TS2339: Property 'ruleName' does not exist on type 'Rule'.
projects/testing-library/migrations/4_0_0/rules/noComponentPropertyRule.ts
(2,49): error TS2307: Cannot find module 'tslint' or its corresponding type declarations.
projects/testing-library/migrations/4_0_0/rules/noComponentPropertyRule.ts
(19,69): error TS2339: Property 'ruleName' does not exist on type 'Rule'.
projects/testing-library/migrations/4_0_0/rules/noCreateComponentRule.ts
(2,49): error TS2307: Cannot find module 'tslint' or its corresponding type declarations.
projects/testing-library/migrations/4_0_0/rules/noCreateComponentRule.ts
(27,69): error TS2339: Property 'ruleName' does not exist on type 'Rule'.
projects/testing-library/migrations/4_0_0/rules/noCreateComponentRule.ts
(37,69): error TS2339: Property 'ruleName' does not exist on type 'Rule'.
projects/testing-library/migrations/5_1_2/rules/noAngularExtensionsImportRule.ts
(2,49): error TS2307: Cannot find module 'tslint' or its corresponding type declarations.
projects/testing-library/migrations/5_1_2/rules/noAngularExtensionsImportRule.ts
(23,69): error TS2339: Property 'ruleName' does not exist on type 'Rule'.

Checklist

Tests
Check / improve the ESLint rules
- Are there any more duplicates which can be removed?
Documentation
- Adjust documentation on testing-library to the github examples
  - src is added as root of example-app -> apps/example-app/app/examples/src instead of apps/example-app/app/examples
Ready to merge

Use NX migrations to upgrade NX. Upgrades Angular to 11.2.9

To be more in line with the NX defaults, the application files are moved to the ./src folder. The tslint-to-eslint migration was failing. Might have been due to being a flattened project.

Remove all angular-eslint/recommended rules. These are provided by the @nrwl/nx plugin fix(eslint): remove duplicate rules Remove all eslint/recommended rules. These are provided by the @nrwl/nx plugin

Somehow the npm run format does not fix all prettier issues. Therefore ran prettier separate. Added the .prettierignore file

New name is `allowedNonPeerDependencies`

Update deprecated imports

remove duplicate rules from: - 'eslint:recommended', - 'plugin:@typescript-eslint/eslint-recommended',

apps/example-app/src/app/examples/01-nested-component.ts

projects/testing-library/src/lib/models.ts

timdeschryver

I noticed that the "ready to merge" checkbox isn't checked.
What would still need to be done in order to get this merged?

From my point of view, this is looking great already and I would lean towards removing TSLint and the migrations.

One remark, there's a link to the example app in the README, that should be updated to the new location.

the-ult · 2021-04-14T18:45:04Z

Wasn't sure I could check it or the reviewer (you) should 😇
I'll try and remove the migration and the TSLint and fix the README

By removing the obsolete TSLint dependency, we need to remove the migrations as well. Since these depend on TSLint specific properties/types BREAKING CHANGE: migrations for v4.0.0 and v5.1.2 are no longer available

the-ult · 2021-04-14T19:11:01Z

BTW:
I tried updating the:

@testing-library/dom          7.29.4  7.29.4  7.30.3  node_modules/@testing-library/dom         angular-testing-library
@testing-library/user-event   12.8.3  12.8.3  13.1.2  node_modules/@testing-library/user-event  angular-testing-library

but these updates caused issues with fakeAsync etc. So I thought it would be best to leave these updates to separate PR by someone with more knowledge of the testing-library 😇

For example: Updating 'only' @testing-library/dom to 7.30.3 results in:

 FAIL   ATL  projects/testing-library/tests/wait-for.spec.ts
  ● allows to override options

    expect(received).rejects.toThrow()

    Received promise resolved instead of rejected
    Resolved to value: <div>Success</div>

      34 |   fireEvent.click(screen.getByTestId('button'));
      35 |
    > 36 |   await expect(waitForATL(() => screen.getByText('Success'), { timeout: 200 })).rejects.toThrow(
         |         ^
      37 |     /Unable to find an element with the text: Success/i,
      38 |   );
      39 | });

      at expect (../../node_modules/expect/build/index.js:134:15)
      at tests/wait-for.spec.ts:36:9
      at fulfilled (../../node_modules/tslib/tslib.js:114:62)
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invoke (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:407:30)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3765:43)
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invoke (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:406:56)
      at Zone.Object.<anonymous>.Zone.run (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:167:47)
      at ../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:1325:38
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invokeTask (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:441:35)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvokeTask (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:3796:43)
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invokeTask (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:440:64)
      at Zone.Object.<anonymous>.Zone.runTask (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:212:51)
      at drainMicroTaskQueue (../../node_modules/zone.js/bundles/zone-testing-bundle.umd.js:627:39)

the-ult · 2021-04-14T19:22:24Z

Should we/somebody update the documentation on the testing-library/docs page after merging?

timdeschryver · 2021-04-14T20:36:20Z

Yep, those failing tests with the latest version of dom testing library are on our radar (that's why it's currently pinned).
I'll give this another look tomorrow morning, but I think this can be merged 🎉
Thanks in advance!

Once it's merged, we'll update the docs.

the-ult · 2021-04-15T06:30:39Z

After it is merged I'll start the PR with the testing-library eslint rules 👍

timdeschryver · 2021-04-15T10:56:47Z

Thanks @the-ult !
@all-contributors please add @the-ult for code

allcontributors · 2021-04-15T10:56:55Z

@timdeschryver

I've put up a pull request to add @the-ult! 🎉

the-ult · 2021-04-15T13:03:37Z

@timdeschryver I adjusted the url on testing-library.com as well.

github-actions · 2021-04-22T17:38:25Z

🎉 This PR is included in version 10.6.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

arjen.althoff and others added 20 commits April 12, 2021 16:45

build(nx): upgrade nx to 12.0.1

e44fc25

Use NX migrations to upgrade NX. Upgrades Angular to 11.2.9

build(deps): update ts-jest

2d3971e

build(deps): update zone.js to 0.11.4

e32c21d

build(example): add strict angular compiler settings

432749d

refactor(example-app): move application code to src folder

eabf881

To be more in line with the NX defaults, the application files are moved to the ./src folder. The tslint-to-eslint migration was failing. Might have been due to being a flattened project.

feat(testing-library): migrate tslint to eslint

6eab313

feat(jest-utils): migrate tslint to eslint

4c8b1dd

feat(example-app): migrate tslint to eslint

b30be1f

build(deps): remove obsolete tslint

3e654e3

build(script): add nx format scripts

94ac99c

fix(eslint): remove duplicate rules

d378f1f

Remove all angular-eslint/recommended rules. These are provided by the @nrwl/nx plugin fix(eslint): remove duplicate rules Remove all eslint/recommended rules. These are provided by the @nrwl/nx plugin

style(all): run prettier

1210da3

Somehow the npm run format does not fix all prettier issues. Therefore ran prettier separate. Added the .prettierignore file

fix(lint): fix eslint errors

3299992

build(nx): upgrade nx to 12.0.3

b6af1ff

build(deps): update eslint dependencies

94ea659

build(deps): re-add tslint due to failing migrations build

9de72d2

build(package): rename deprecated whitelistedNonPeerDependencies

9798723

New name is `allowedNonPeerDependencies`

style: run format

3030207

build(deps): upgrade jest-preset-angular

7917215

Update deprecated imports

fix(eslint): remove duplicate rules

36aa466

remove duplicate rules from: - 'eslint:recommended', - 'plugin:@typescript-eslint/eslint-recommended',

the-ult commented Apr 13, 2021

View reviewed changes

apps/example-app/src/app/examples/01-nested-component.ts Show resolved Hide resolved

the-ult commented Apr 13, 2021

View reviewed changes

projects/testing-library/src/lib/models.ts Show resolved Hide resolved

timdeschryver reviewed Apr 14, 2021

View reviewed changes

the-ult added 3 commits April 14, 2021 20:50

build(ng): update angular material and cdk to 11.2.9

2fcacfe

docs: change url for See more examples

c5b95e9

build: remove tslint and migrations

1a2f75e

By removing the obsolete TSLint dependency, we need to remove the migrations as well. Since these depend on TSLint specific properties/types BREAKING CHANGE: migrations for v4.0.0 and v5.1.2 are no longer available

the-ult changed the title ~~WIP: feat: migrate tslint to eslint~~ feat: migrate tslint to eslint Apr 14, 2021

timdeschryver approved these changes Apr 15, 2021

View reviewed changes

timdeschryver merged commit a9383fa into testing-library:master Apr 15, 2021

timdeschryver changed the title ~~feat: migrate tslint to eslint~~ chore: migrate tslint to eslint Apr 15, 2021

allcontributors bot mentioned this pull request Apr 15, 2021

docs: add the-ult as a contributor #200

Merged

the-ult deleted the feat/migrate-tslint-to-eslint branch April 15, 2021 11:07

the-ult mentioned this pull request Apr 15, 2021

docs(testing-library-angular): Adjust url of the github example-app testing-library/testing-library-docs#795

Merged

github-actions bot added the released label Apr 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: migrate tslint to eslint #199

chore: migrate tslint to eslint #199

Uh oh!

the-ult commented Apr 13, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

timdeschryver left a comment •

edited

Loading

Uh oh!

the-ult commented Apr 14, 2021

Uh oh!

the-ult commented Apr 14, 2021 •

edited

Loading

Uh oh!

the-ult commented Apr 14, 2021

Uh oh!

timdeschryver commented Apr 14, 2021 •

edited

Loading

Uh oh!

the-ult commented Apr 15, 2021

Uh oh!

timdeschryver commented Apr 15, 2021

Uh oh!

allcontributors bot commented Apr 15, 2021

Uh oh!

the-ult commented Apr 15, 2021

Uh oh!

github-actions bot commented Apr 22, 2021

Uh oh!

Uh oh!

chore: migrate tslint to eslint #199

chore: migrate tslint to eslint #199

Uh oh!

Conversation

the-ult commented Apr 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Troubleshooting

Checklist

Uh oh!

Uh oh!

Uh oh!

timdeschryver left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-ult commented Apr 14, 2021

Uh oh!

the-ult commented Apr 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-ult commented Apr 14, 2021

Uh oh!

timdeschryver commented Apr 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-ult commented Apr 15, 2021

Uh oh!

timdeschryver commented Apr 15, 2021

Uh oh!

allcontributors bot commented Apr 15, 2021

Uh oh!

the-ult commented Apr 15, 2021

Uh oh!

github-actions bot commented Apr 22, 2021

Uh oh!

Uh oh!

the-ult commented Apr 13, 2021 •

edited

Loading

timdeschryver left a comment •

edited

Loading

the-ult commented Apr 14, 2021 •

edited

Loading

timdeschryver commented Apr 14, 2021 •

edited

Loading