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

request: update eslint to v7 #905

Open
cfanoulis opened this issue Oct 15, 2020 · 7 comments
Open

request: update eslint to v7 #905

cfanoulis opened this issue Oct 15, 2020 · 7 comments
Labels
scope: dependencies Pull requests that update a dependency file solution: workaround available There is a workaround available for this issue

Comments

@cfanoulis
Copy link

Current Behavior

tsdx is currently using version 6.8 of eslint, which is quite outdated and breaks most eslint configurations (for example: @skyra/eslint-config

Desired Behavior

tsdx uses a newer version of eslint

Suggested Solution

Migrate to the newest version of eslint - currently 7.11.0

Who does this impact? Who is this for?

This issue mostly impacts users that either want to use or are migrating projects that are using newer eslint rules

Describe alternatives you've considered

Using the resolution field - This is a fragile solution and may cause problems with the internal config

Additional context

N/A

@agilgur5 agilgur5 added progress: blocked scope: dependencies Pull requests that update a dependency file solution: workaround available There is a workaround available for this issue labels Oct 15, 2020
@agilgur5
Copy link
Collaborator

Unfortunately, this is blocked for the same reason as #810, eslint-config-react-app has yet to release an upgraded version per #810 (comment)

quite outdated

ESLint 7 was only released in May, so I wouldn't say it's that outdated, especially not for a major that was out for nearly a year.

breaks most eslint configurations

I'm not sure what you mean by this or what it breaks. It doesn't seem like ESLint 7 had too many breaking changes.

Using the resolution field - This is a fragile solution and may cause problems with the internal config

Yes, it should work in the meantime if you need it though. One can also fully replace tsdx lint with plain ESLint 7 if one wants.

@JustFly1984
Copy link

@agilgur5 that is exactly the case why I'm against eslint-config-react-app and any manageable eslint configs. Authors do not maintain them frequently. All those eslint plugins have outdated dependencies, and even could have security issues.

Authors of eslint dependencies constantly adding new useful features and rules, fixing bugs and improve security. Updating this dependencies often, helps to maintain code quality and security in dependency tree.

The main issue for me in updating eslint and rules in open source repos - it actually can leave a huge github commit due to some styling rules, it will make it harder to use git blame, and authoring for code lines will go to the author of new commit - me in this case. I could setup eslint config with rules in PR.

I will make sure that config works, but you will need to run eslint on project and fix/autofix everything it will find.

If it is fine for you - I can make PR.

@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 16, 2020

and even could have security issues.

If you mean vulnerabilities, they don't actually have any, so that's a strawman. Shareable configs often are just plain JSON -- there's really no space for a vulnerability there (and I've worked as a Security Engineer before). I've also fixed all of the relevant vulnerability warnings in TSDX and it currently shows none.

Authors of eslint dependencies constantly adding new useful features and rules, fixing bugs and improve security. Updating this dependencies often, helps to maintain code quality and security in dependency tree.

I'm not sure what you mean by "dependencies", it sounds like you're using "dependencies", "plugins", and "configs" interchangeably but those are 3 distinct terms.

In any case, I think you're missing the forest for the trees here -- most of those changes are breaking changes and that's one reason why maintaining shareable configs is not easy. Someone has to do all that updating, deciding which rules to use, and figuring out what is breaking and what is isn't. Also many rules are stylistic, those don't improve code quality by definition (although code quality itself is a subjective metric).

@JustFly1984 As I wrote to you in #182 (comment) where I linked to #890 (comment), TSDX already has a problem with having too many dependencies / doing too many things and therefore requiring lots of breaking changes (I've been putting in a lot of effort in heavily batching them so users don't feel this as much). I'm repeating myself here one more time, but I wrote there that this does not make it easy to maintain and adding a custom rule set would make that even harder and would add even more breaking changes. I don't think that's a good idea and I also personally do not want to make my role as a maintainer even harder or give myself another huge amount of work and maintenance burden. Please recognize what kind of ask you're repeatedly making.
I think I'm doing quite a lot as is and making something continuously harder to maintain is a strategy to burnout all maintainers and kill a library. Having too many breaking changes is also a strategy to alienate users or have them behind many versions. In contrast, I'm focusing my time and planning on ways to make it easier to maintain and easier to opt-out or use your own.

but you will need to run eslint on project and fix/autofix everything it will find.

It sounds like you want to propose a different configuration as well, which would be a very breaking change. As I wrote in the linked comment to you, if you want to propose another config, please make an RFC. Adding comments to different issues is not the same thing. As I wrote there, that is not a small change and should be very carefully considered before any PR is even started, especially as TSDX users have overlap with CRA users.

Also, as I wrote here, you don't have to use tsdx lint and can use plain eslint with your own configs etc. tsdx lint can't by definition fulfill everyone's needs and its main purpose is to enable zero config.

@cfanoulis
Copy link
Author

@agilgur5 thanks for the (in-depth) answer, I expected resolutions may have broken something internal. Is there any (even vague is enough) timeline for when the big batch of breaking changes may land?

@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 16, 2020

@cfanoulis this is slated for v0.16.0 per the milestone, which I don't think will be for another month or two. But this can't be completed right now in any case, it's blocked by upstream, per my first comment's first sentence:

Unfortunately, this is blocked for the same reason as #810, eslint-config-react-app has yet to release an upgraded version per #810 (comment)

I don't know when eslint-config-react-app will release an update; that's the key unknown in the timeline

@jonnyelliot
Copy link

I think this is now unblocked. eslint-config-react-app have released a new version using eslint 7.5!

@agilgur5
Copy link
Collaborator

agilgur5 commented Nov 7, 2020

Yep, saw it came out of pre-release recently, so now it's just about getting to v0.16.0. I'm looking to release v0.15.0 soon (really depends on how much time I have as there's a few PRs I need to finish up for v0.14.2 and v0.15.0; I had planned to release it earlier) and I try to give at least a month between breaking changes to not overload users. Per v0.14.0's release notes, I originally thought of combining several breaking changes in one, but other than this blocker, the release notes became so long that I thought it'd be super confusing to users. I've already drafted part of the release notes for v0.15.0 and v0.16.0 as a result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: dependencies Pull requests that update a dependency file solution: workaround available There is a workaround available for this issue
Projects
None yet
Development

No branches or pull requests

4 participants