feat: Add Eslint to sentry-javascript #2786
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report
|
AbhiPrasad
force-pushed
the
abhi/feat/eslint
branch
from
a877ce0
to
cf7a9f1
Compare
AbhiPrasad
commented
Jul 31, 2020
| "eslint": "^7.5.0", | ||
| "eslint-config-prettier": "^6.11.0", | ||
| "eslint-plugin-jsdoc": "^30.0.3", | ||
| "eslint-plugin-sentry-sdk": "file:./eslint-plugin-sentry-sdk", |
There was a problem hiding this comment.
All of these dependencies are temporary. When we move the main eslint config out of this repo, we should be able to replace this (at the package level) with just a single eslint-config-sentry-sdks
AbhiPrasad
force-pushed
the
abhi/feat/eslint
branch
from
350b947
to
d26de38
Compare
HazAT
approved these changes
Aug 3, 2020
| "chai": "^4.1.2", | ||
| "codecov": "^3.6.5", | ||
| "danger": "^7.1.3", | ||
| "danger-plugin-eslint": "^0.1.0", | ||
| "danger-plugin-tslint": "^2.0.0", |
There was a problem hiding this comment.
Yes, because the other packages still use tslint. We can remove after we are done.
|
Sooooooooo good, thank you! |
kamilogorek
pushed a commit
that referenced
this pull request
Aug 10, 2020
This PR adds an initial implementation of an eslint-config to the mono-repo. It also adds a eslint-plugin-sentry-sdks, a local npm package we use to write our own custom eslint rules, and converts @sentry/browser from eslint to tslint.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Total changes after disregarding
yarn.lock: +295 -510.See Asana ticket: https://app.asana.com/0/1159797622087522/1186701350284441/f
This PR adds an initial implementation of an
eslint-configto the mono-repo. It also adds aeslint-plugin-sentry-sdks, a local npm package we use to write our own custom eslint rules, and converts@sentry/browserfrom eslint to tslint.Here is how this works. I've created an
.eslintignorethat has all the packages that we ignore (everything except@sentry/browserright now). As I convert packages, I'll be updating and eventually deleting that file.The heart of the matter is in the
.eslintrc.jsfile located at the heart of the repo. This specifies all the rules that will eventually go intoeslint-config-sentry-sdks. I've commented each rule with justification, so go take a look. This maps fairly 1 to 1 with our old tslint config, but I took the liberty with changing some rules.This
eslintrc.jsfile in the root of the repo also relies on a neweslint-plugin-sentry-sdks. This plugin currently only implements one rule, to preventasync awaitusage, but I expect to add more as I convert the rest of the packages.The plan for conversion is that I'll address all the packages in the mono-repo, and then move the config out of the repo.
In terms of
@sentry/browserchanges:You'll notice some class methods moved around. This is based on using the default from https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-ordering.md which I think is better suited to organize classes than what we had before.
I typed
any->unknownwhere I was 100% confident, but for the rest I just slammed theeslint-disable-next-line @typescript-eslint/no-explicit-any. We should add a task after this is done to review all ourno-explicit-anyusage.I changed some regex because it eslint warned about unnecessary escape.
I removed all instances of tslint, and the
tslint.jsonin favour of an.eslintrc.jsfile