Skip to content
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

[RUMF-324] Replace TSLint with ESLint #681

Merged
merged 73 commits into from
Jan 26, 2021
Merged

[RUMF-324] Replace TSLint with ESLint #681

merged 73 commits into from
Jan 26, 2021

Conversation

webNeat
Copy link
Contributor

@webNeat webNeat commented Jan 12, 2021

Motivation

TSLint is deprecated and ESLint is the recommended alternative.

Changes

  • Generated an ESLint config with the same rules as our TSLint using tslint-to-eslint-config
  • Disabled the rules that don't pass on our code.
  • Fixed the auto-fixable rules.

Testing


I have gone over the contributing documentation.

@webNeat webNeat requested a review from a team as a code owner January 12, 2021 15:19
@codecov-io
Copy link

codecov-io commented Jan 12, 2021

Codecov Report

Merging #681 (7b9c6d8) into master (e2d7bae) will increase coverage by 0.28%.
The diff coverage is 77.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   79.60%   79.89%   +0.28%     
==========================================
  Files          68       68              
  Lines        3433     3407      -26     
  Branches      737      732       -5     
==========================================
- Hits         2733     2722      -11     
+ Misses        700      685      -15     
Impacted Files Coverage Δ
packages/core/src/domain/configuration.ts 76.00% <0.00%> (+2.92%) ⬆️
packages/core/src/tools/context.ts 85.10% <ø> (ø)
packages/core/src/transport/transport.ts 91.66% <ø> (ø)
...ages/rum-core/src/browser/domMutationCollection.ts 89.47% <ø> (ø)
...ages/rum-core/src/browser/performanceCollection.ts 17.04% <0.00%> (-0.20%) ⬇️
packages/rum-core/src/domain/assembly.ts 100.00% <ø> (ø)
packages/rum-core/src/domain/lifeCycle.ts 100.00% <ø> (ø)
...omain/rumEventsCollection/error/errorCollection.ts 100.00% <ø> (ø)
packages/rum-core/src/domain/tracing/tracer.ts 100.00% <ø> (ø)
packages/rum-recorder/src/domain/rrweb/types.ts 100.00% <ø> (ø)
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2d7bae...7b9c6d8. Read the comment docs.

.eslintignore Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
packages/core/src/domain/tracekit.spec.ts Outdated Show resolved Hide resolved
.eslintignore Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
packages/core/src/domain/tracekit.spec.ts Outdated Show resolved Hide resolved
packages/core/test/capturedExceptions.ts Outdated Show resolved Hide resolved
LICENSE-3rdparty.csv Outdated Show resolved Hide resolved
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the .eslintrc.js could be simplified further by:

  • removing rules that are already enabled or disabled by some extended config (extends field). For example,

  • removing disabled rules that are not enabled or undeclared. For example, jsdoc/check-indentation is not enabled (since we don't extend plugin:jsdoc/recommended and all rules are disabled by default, we are sure this is disabled) and could be removed.

  • removing rules configuration for disabled rules

  • grouping rules together (it seems that they are ordered by name size, it would be more useful to group by plugin and then alphabetically or disabled first/enabled second, for example)

.eslintrc.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
packages/core/src/domain/tracekit.spec.ts Outdated Show resolved Hide resolved
packages/core/src/tools/context.spec.ts Outdated Show resolved Hide resolved
@webNeat
Copy link
Contributor Author

webNeat commented Jan 18, 2021

@BenoitZugmeyer I agree with you that the ESLint config file still needs cleanup. Can we postpone that until we decide about all rules and enable all the ones we want to keep?

@BenoitZugmeyer
Copy link
Member

Since it will directly impact our dev setup, I'd prefer to merge a clean and final PR instead of merging a half-baked PR and postponing configuration changes.

Copy link
Member

@BenoitZugmeyer BenoitZugmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me! Great work!

Copy link
Contributor

@bcaudan bcaudan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@webNeat webNeat merged commit aa97fdf into master Jan 26, 2021
@webNeat webNeat deleted the amine/eslint branch January 26, 2021 09:44
kcaffrey pushed a commit to WonderInventions/browser-sdk that referenced this pull request Jan 29, 2021
* 🔨 Replace TSLint by ESLint

* 🔨 Fix rule import/order

* 🔨 Fix rule arrow-body-style

* 🔨 Fix rule @typescript-eslint/dot-notation

* 🔨 Fix rule @typescript-eslint/no-unnecessary-type-assertion

* Update licences of 3rd party dependencies

* 🎨 Fix prettier format

* 🔨 Ignore special cases of @typescript-eslint/no-unnecessary-type-assertion

* 👌 Add new line at the end of file

* 👌 Reuse tsconfig files for ESLint

* 👌 Enable ESLint for packages/core/src/domain/tracekit.spec.ts

* 👌 Ignore test/app/dist and sandbox from ESLint

* 👌 Remove unused plugins

* 👌 Refactor ESLint comments

* Ignore coverage directory

* 🔨 Fix rule @typescript-eslint/array-type

* 🔨 Fix rule @typescript-eslint/no-shadow

* 🔨 Fix rule @typescript-eslint/no-redeclare

* 🔨 Fix rule no-throw-literal

* 🔨 Fix rule unicorn/filename-case

* 🔨 Fix rule jsdoc/check-indentation

* 🔨 Fix rule @typescript-eslint/await-thenable

* 🔨 Fix rule @typescript-eslint/no-unused-vars

* 🔨 Fix rule @typescript-eslint/ban-ts-comment

* 🔨 Fix rule @typescript-eslint/no-unsafe-call

* 🔨 Fix rule @typescript-eslint/no-for-in-array

* 🔨 Fix rule @typescript-eslint/member-ordering

* 🔨 Fix rule @typescript-eslint/no-var-requires

* 🔨 Fix rule @typescript-eslint/no-unsafe-return

* 🔨 Fix rule no-underscore-dangle

* 🔨 Fix rule @typescript-eslint/require-await

* 🔨 Fix rule @typescript-eslint/no-empty-interface

* 🔨 Fix rule @typescript-eslint/prefer-regexp-exec

* 🔨 Fix rule @typescript-eslint/no-inferrable-types

* 🔨 Fix rule @typescript-eslint/no-floating-promises

* 🔨 Fix rule @typescript-eslint/restrict-plus-operands

* 🔨 Fix rule @typescript-eslint/restrict-template-expressions

* 🔨 Fix rule @typescript-eslint/ban-types

* 🔨 Disable rule no-param-reassign

* 🔨 Fix rule @typescript-eslint/unbound-method

* 👌 Sort ESLint rules

* 🎨 Remove rules already on @typescript-eslint/recommended

* Fix ESLint errors

* 🔨 Remove Useless TSLint comments

* Remove uneeded operators and comments

Co-authored-by: Bastien Caudan <[email protected]>
Co-authored-by: Benoît <[email protected]>

* Use Regexp.test() instead of .exec()

Co-authored-by: Benoît <[email protected]>

* 👌 Remove useless void operator

* 👌 Handle the promise correctly

* 👌 Ignore the rule for the entire file

* 👌 Remove useless type cast

* 👌 Restore original monitor implementation

* 👌 Remove unused function

* 👌 Remove unused rule

* 👌 Refactor jsdoc comments

* 👌 Ignore the rule for the entire test file

* 👌 Ignore the rule only when needed

* 👌 Regenrate rumEvent.types

* 👌 Remove uneeded comment

* 👌 Log fetch errors

* 👌 Handle fetch promise error

* 👌 Simplify tsconfigs

* 👌 Undo uneeded change

* Fix typescript config

* 👌 Use spaced comments

* 👌 Disable all rules for deflateWorker.js

* 👌 Remove uneeded comment

* Fix lint on CI

* 👌 Improve tests readability

* Remove unused import

Co-authored-by: Bastien Caudan <[email protected]>
Co-authored-by: Benoît <[email protected]>
kcaffrey added a commit to WonderInventions/browser-sdk that referenced this pull request Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants