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

Decide which Node versions ESLint 6 should support #11456

Closed
not-an-aardvark opened this issue Mar 1, 2019 · 8 comments · Fixed by #11557
Closed

Decide which Node versions ESLint 6 should support #11456

not-an-aardvark opened this issue Mar 1, 2019 · 8 comments · Fixed by #11557
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features
Projects

Comments

@not-an-aardvark
Copy link
Member

This issue is a follow-up from today's TSC meeting. The TSC decided that ESLint 6 will drop support for Node 6, with the precise Node version support TBD.

Based on #11022, it seemed like the consensus was that we should pick Node version requirements based on specific features we might want to use in ESLint, rather than just picking the latest Node versions as we did for v5.x. (However, not a lot of team members have chimed in on #11022 -- feel free to add to that discussion, particularly if you disagree with that general approach.) I've mostly gone with that approach, although I've also taken into consideration which Node versions users will typically be upgrading to.


We currently support everything above Node 8.10.0 on the Node 8 release line. I don't think we should tighten that requirement, since AWS Lambda only supports Node 8.10.0. We've received a few requests to loosen that requirement with ESLint 5, but I'd rather not do that either since it could be nice to have features like Module.builtinModules and correct require.resolve behavior with the paths argument, both of which were introduced in Node 8.10.0.

I think we should drop support for Node 9, since it's been at EOL for almost a year.

We currently support all versions of Node 10. There aren't any features we specifically need in Node 10 aside from the ones that in Node 8, so I'd recommend starting at the first Node 10 LTS release (i.e. ^10.13.0) since this is probably where most users start using Node 10.

Node 11 is going to be EOL in June, so I don't think too many users will be stuck on old versions of it. As a result, I'd arbitrarily suggest picking the latest version as of now, i.e. 11.10.1.

So I think our Node version support for ESLint 6 should be:

^8.10.0 || ^10.13.0 || >=11.10.1

@not-an-aardvark not-an-aardvark added core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion needs bikeshedding Minor details about this change need to be discussed breaking This change is backwards-incompatible labels Mar 1, 2019
@not-an-aardvark not-an-aardvark added this to Accepted, ready to implement in v6.0.0 Mar 1, 2019
@ilyavolodin
Copy link
Member

I think 8.10 was the exception out of the rule because of the introduction of new features that were fixing broken behavior. For other versions, I don't see the point of being too specific. I would be fine with anything greater then 10 or 11.

@not-an-aardvark
Copy link
Member Author

Well, every Node release line includes bugfixes, so in that sense we're in the same situation now as we were with 8.x. It wasn't originally supposed to be a special exception.

I think it would be nice to make our Node requirement specific by excluding infrequently-used versions, since it reduces the likelihood of inadvertently introducing bugs on supported versions. (With our current Travis setup, we only test the latest Node release on each release line anyway, so if a bug were introduced that only affects 10.0.0, we probably wouldn't notice. We've discussed adding builds explicitly for versions like 10.0.0 in the past, but there were concerns about builds taking too long.)

However, I don't feel strongly about it and would be fine with something like ^8.10.0 || >=10.0.0 if other people are opposed to making the requirements so specific.

@ilyavolodin
Copy link
Member

I mean Node 8.10 was a special case, because our code wasn't working with versions <8.10. While something like that might happen again, it's unlikely, since those fixes were more like breaking changes.

@not-an-aardvark
Copy link
Member Author

not-an-aardvark commented Mar 6, 2019

Are you sure? From what I recall, our code was working fine with Node <8.10, but we chose to only support things above that version in case we wanted to use that version's features in the future.

@ilyavolodin
Copy link
Member

Hmm... You might be right. I'm a bit hazy on the details of that conversation.

@nzakas
Copy link
Member

nzakas commented Mar 13, 2019

I'm 👍 to ^8.10.0 || ^10.13.0 || >=11.10.1

@mysticatea mysticatea moved this from Accepted, ready to implement to Implemented, pending review in v6.0.0 Mar 27, 2019
@not-an-aardvark not-an-aardvark added the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Mar 28, 2019
@not-an-aardvark
Copy link
Member Author

TSC Summary: This issue proposes setting our Node version support in ESLint 6 to ^8.10.0 || ^10.13.0 || >=11.10.1.

TSC Question: Should we accept this proposal?

@btmills btmills removed the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Mar 28, 2019
@btmills
Copy link
Member

btmills commented Mar 28, 2019

During today's TSC meeting, we approved the minimum Node versions as ^8.10.0 || ^10.13.0 || >=11.10.1.

@mysticatea mysticatea moved this from Implemented, pending review to Ready to merge in v6.0.0 Mar 29, 2019
@mysticatea mysticatea removed the needs bikeshedding Minor details about this change need to be discussed label Mar 29, 2019
v6.0.0 automation moved this from Ready to merge to Done Apr 1, 2019
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 29, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features
Projects
No open projects
v6.0.0
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants