peerDeps should use ^ range type

[mightyiam]

I don't know why we're using >=. That claims support for larger major versions, which isn't a reasonable claim. The change seems to come from 6c8c04c, which was me, but it doesn't explain much other than referring to #94 which also doesn't explain much. I feel that the peerDeps should use ^. That's a breaking change.

[theoludwig]

I don't know why we're using >=. That claims support for larger major versions, which isn't a reasonable claim.

Actually, it is a reasonable claim, we should always use the last up-to-date version of dependencies and not stay behind, it is fine to use >=.

I feel that the peerDeps should use ^. That's a breaking change.

Indeed, if we now use ^ instead of >= it is a BREAKING CHANGE.
This BREAKING CHANGE is not necessary and I'm against it for the reasons I mentioned earlier.

---

Note that even if do use ^, it will not solve the test failures, which will be fixed thanks to #590.
We should support at least the same peerDependencies as eslint-config-standard support.

Currently the peerDependencies of eslint-config-standard are :

  "peerDependencies": {
    "eslint": "^7.12.1",
    "eslint-plugin-import": "^2.22.1",
    "eslint-plugin-node": "^11.1.0",
    "eslint-plugin-promise": "^4.2.1 || ^5.0.0"
  },

https://github.com/standard/eslint-config-standard/blob/2cfcbfd91bb20f3479cd0e9b425324746eb3554b/package.json#L49-L54

The peerDependencies causing problem is this one : "eslint-plugin-promise": "^4.2.1 || ^5.0.0" as we're not checking correctly, because our peerDependencies is actually satisfying this one : "eslint-plugin-promise": ">=4.2.1" but in the test we are not checking that.

We should consider merging #590, it is much cleaner to check if our peerDependencies is satisfying the peerDependencies of eslint-config-standard than strict equality, that would avoid to have the tests random failing because eslint-config-standard releases a new version with a peerDependencies change as it is currently the case. @mightyiam

[mightyiam]

I don't know why we're using >=. That claims support for larger major versions, which isn't a reasonable claim.

Actually, it is a reasonable claim, we should always use the last up-to-date version of dependencies and not stay behind, it is fine to use >=.

I feel it's not reasonable, because a major version of a peer dep is likely to be incompatible with the config this package provides for the rules which that peer dep provides.

I feel that the peerDeps should use ^. That's a breaking change.

Indeed, if we now use ^ instead of >= it is a BREAKING CHANGE.
This BREAKING CHANGE is not necessary and I'm against it for the reasons I mentioned earlier.

Breaking changes in this package used to be quite frequent while I was actively following typescript-eslint releases and adding new rules from it here. I would say that well documented breaking changes are nothing to be avoided. Well, not this one, because it's a bug fix.

Note that even if do use ^, it will not solve the test failures, which will be fixed thanks to #590.

Yes, that's true.

We should support at least the same peerDependencies as eslint-config-standard support.

Yes, generally, yes.

Currently the peerDependencies of eslint-config-standard are :

  "peerDependencies": {
    "eslint": "^7.12.1",
    "eslint-plugin-import": "^2.22.1",
    "eslint-plugin-node": "^11.1.0",
    "eslint-plugin-promise": "^4.2.1 || ^5.0.0"
  },

https://github.com/standard/eslint-config-standard/blob/2cfcbfd91bb20f3479cd0e9b425324746eb3554b/package.json#L49-L54

The peerDependencies causing problem is this one : "eslint-plugin-promise": "^4.2.1 || ^5.0.0" as we're not checking correctly, because our peerDependencies is actually satisfying this one : "eslint-plugin-promise": ">=4.2.1" but in the test we are not checking that.

We should consider merging #590, it is much cleaner to check if our peerDependencies is satisfying the peerDependencies of eslint-config-standard than strict equality, that would avoid to have the tests random failing because eslint-config-standard releases a new version with a peerDependencies change as it is currently the case. @mightyiam

I will submit a PR with a proposed solution.

[voxpelli]

I agree that >= isn't correct, the ^ is the correct one, though the >= was historically used in some repos in the days when npm enforced peer dependencies, as one could else get stuck in nasty situations. Since npm once again starts to enforce peer dependencies now with npm 7.x it's a tricky one, but staying consistent with the rest of the standardecosystem and moving to ^ would probably be a good strategy for now 👍

[theoludwig]

Right, I'm fine with ^.
Let's do that then. 👍

[theoludwig]

The CI fails as discussed in request changes of the PR #597, we should support "eslint-plugin-promise": "^4.2.1 || ^5.0.0",. @mightyiam

[mightyiam]

@divlo thanks. That's a different issue and I'm on it. I feel we can close this one though.