Skip to content

Commit

Permalink
[eslint] bump minimum v7 version to v7.2.0
Browse files Browse the repository at this point in the history
  • Loading branch information
@ljharb
ljharb committed Jun 7, 2020
1 parent 199143c commit d84062e
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [Docs] `order`: fix bad inline config ([#1788], thanks [@nickofthyme])
- [Tests] Add fix for Windows Subsystem for Linux ([#1786], thanks [@manuth])
- [Docs] `no-unused-rules`: Fix docs for unused exports ([#1776], thanks [@barbogast])
- [eslint] bump minimum v7 version to v7.2.0

## [2.20.2] - 2020-03-28
### Fixed
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
"chai": "^4.2.0",
"coveralls": "^3.0.6",
"cross-env": "^4.0.0",
"eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.0.0-0",
"eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0",
"eslint-import-resolver-node": "file:./resolvers/node",
"eslint-import-resolver-typescript": "^1.0.2",
"eslint-import-resolver-webpack": "file:./resolvers/webpack",
Expand All @@ -94,7 +94,7 @@
"typescript-eslint-parser": "^22.0.0"
},
"peerDependencies": {
"eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.0.0-0"
"eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0"
},
"dependencies": {
"array-includes": "^3.1.1",
Expand Down

6 comments on commit d84062e

@jaydenseric
Copy link

Choose a reason for hiding this comment

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

@ljharb what was the reason for this change?

@ljharb
Copy link
Member Author

@ljharb ljharb commented on d84062e Jan 12, 2021

Choose a reason for hiding this comment

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

By the time this was ready for release, there was no point supporting older versions of eslint 7. It’s always best to require as new a version of a major line as one can without a breaking change.

@jaydenseric
Copy link

Choose a reason for hiding this comment

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

Thanks. So eslint v7 - 7.2 quite possibly works? I need to know if I need to increase my own peer dependency range from v7 to v7.2, which would be a breaking change I would rather avoid.

@ljharb
Copy link
Member Author

@ljharb ljharb commented on d84062e Jan 12, 2021

Choose a reason for hiding this comment

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

I'm not sure about your question. If you're using eslint-plugin-import as a direct dep/dev dep, then you must use eslint v7.2 or above. If you're using it as a peer dep (what's your use case?), then your consumers also must use v7.2 or above.

Whether it "works" is irrelevant; the peer dep range is an absolute requirement, and npm ls will indicate that eslint v7.0 or v7.1 makes for an invalid dependency graph.

@jaydenseric
Copy link

@jaydenseric jaydenseric commented on d84062e Jan 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

eslint-plugin-import is a peer dependency of eslint-config-env. My new version I haven't pushed up yet will have an eslint ^7.0.0 peer dependency, since I have increased the parserOptions.ecmaVersion to 2021, breaking support for eslint v6. While eslint-plugin-import's eslint v7.2.0 peer dependency will make it impossible for an npm v7 project to install eslint at v7.0.0, my eslint-config-env peer dependency range for eslint ^7.0.0 should not cause a problem as eslint-plugin-import will dictate the version that gets installed as it has a higher requirement.

I have some faint recollections of some environment that could have problems (yarn pnp?) so, I will probably match eslint-plugin-import's eslint v7.2.0 peer dependency requirement. It's annoying that we are declaring eslint v7.0.0 is not supported, when it probably works fine. I disagree we should make peer dependencies as recent as possible in major releases just because you can. Because then it forces the whole ecosystem that might also use your package to abandon the earlier versions in their peer dependency range also.

The eslint-plugin-import peer dependency "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0" reveals you have only applied this approach for eslint v7. By your same through process, why didn't you drop all the earlier eslint major versions at the same time? Why support eslint v6.0.0 but not v7.0.0? I.e, why didn't you update it to:

  • "eslint": "^7.2.0"
  • Or, "eslint": "^2.13.1 || ^3.19 || ^4.19.1 || ^5.16 || ^6.8 || ^7.2.0"

@ljharb
Copy link
Member Author

@ljharb ljharb commented on d84062e Jan 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

That should work fine, but declaring v7.0.0 when your deps require v7.2.0 doesn’t add any value imo.

The desire is to force everyone to be on as late a version as possible, at least for a dev tool like eslint where there’s no reason not to update aggressively.

the reason we didn’t do it on the other majors is simply that we didn’t take as long to release support for them (vX.0.0 was the latest at the time), and eslint wasn’t making releases so quickly back then either. It would also be a breaking change to raise the threshold now, but in our next major release, whenever that is, we will definitely be doing precisely that.

Please sign in to comment.