[proposal] Making most peerDependencies -> dependencies #88
Conversation
| "eslint": ">=6.0.0" | ||
| }, | ||
| "dependencies": { | ||
| "babel-eslint": "*", |
There was a problem hiding this comment.
I love these being regular dependencies but don't love them being * versioned. It's not uncommon to encounter breakages due to differing compatibility with different eslint versions and in the case of babel-eslint babel versions. I'd rather using dependencies tied a semver major range.
There was a problem hiding this comment.
💯 ^1.2.3 + renovate/greenkeeper/etc would go a long way here.
There was a problem hiding this comment.
Sounds good... I'll keep eslint as a peer because the other plugins do that anyway...
I'll update the PR now
There was a problem hiding this comment.
Updated based on the suggestions here.
unrelated: we should also probably create a separate PR removing the es5 package, it's just one more thing to maintain and it's probably not used by anyone.
| }, | ||
| "dependencies": { | ||
| "babel-eslint": "^10.0.3", | ||
| "eslint-config-godaddy": "^4.0.0", |
There was a problem hiding this comment.
I need to use the next version for these, can I have publish permission please?
| "babel-eslint": "^10.0.3", | ||
| "eslint-config-godaddy": "^4.0.0", | ||
| "eslint-plugin-flowtype": "^4.5.2", | ||
| "eslint-plugin-jsx-a11y": "*", |
There was a problem hiding this comment.
Could you change this to a ^ version? Don't want a non-backwards-compatible release breaking things for people.
|
I guess I missed this one, thx!
On Mon, Dec 2, 2019 at 9:00 AM Jacob Page ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In packages/eslint-config-godaddy-react/package.json
<#88 (comment)>:
> "peerDependencies": {
- "babel-eslint": ">=7.2.1 <11.0.0",
- "eslint": "^6.1.0",
- "eslint-plugin-json": "^1.4.0",
- "eslint-plugin-jsx-a11y": "^6.2.3",
- "eslint-plugin-mocha": "^5.3.0",
- "eslint-plugin-react": "^7.14.2"
+ "eslint": ">=6.0.0"
+ },
+ "dependencies": {
+ "babel-eslint": "^10.0.3",
+ "eslint-config-godaddy": "^4.0.0",
+ "eslint-plugin-flowtype": "^4.5.2",
+ "eslint-plugin-jsx-a11y": "*",
Could you change this to a ^ version? Don't want a
non-backwards-compatible release breaking things for people.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#88?email_source=notifications&email_token=AAAG64S2V6GETBSA3DBZV5LQWU5JNA5CNFSM4JQXE23KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNSQQSA#pullrequestreview-325388360>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAG64USDZXDJG5O2NC25ATQWU5JNANCNFSM4JQXE23A>
.
--
--
Fabio Miranda Costa
*twitter:* @fabioMiranda
*github:* fabiomcosta
|
| "eslint-plugin-babel": "^5.3.0", | ||
| "eslint-plugin-flowtype": "^4.2.0", | ||
| "eslint-plugin-json": "^1.4.0", | ||
| "eslint-plugin-mocha": "^5.3.0" |
There was a problem hiding this comment.
I don't think these can be just removed. I do understand the peerDependencies messages might be annoying. This plugin extends from the base:
Iirc this will just lead to eslint crashing on not finding the modules.
There was a problem hiding this comment.
These will now installed as regular dependencies, though, so eslint should be able to find all the relevant plugins, no? It doesn't make sense to me that they're peer dependencies; installing our canned config should install all the relevant plugins for you. All other eslint configs I've seen do this.
There was a problem hiding this comment.
I stand corrected overlooked them here https://github.com/godaddy/javascript/pull/88/files#diff-502dbc4a5275e8ef5e9db3306e47d92cR31
| "eslint-plugin-json": "^1.2.0", | ||
| "eslint-plugin-mocha": "^5.2.0", | ||
| "eslint-plugin-react": "^7.11.0", | ||
| "eslint-plugin-jsx-a11y": "^6.2.1" |
There was a problem hiding this comment.
same here, see comment above. The extend is two levels deep here though and based on the config-react plugin
fabiomcosta
changed the title
Today, when installing any of the
eslint-config-godaddy-*packages, you're also required to install some peer deps that most ppl shouldn't care about.My understanding is that this was done so consumers could more easily control those version on their side and we wouldn't need to have to keep maintaining that on our side.
This PR tries to make installing these packages easier, without showing the annoying warnings about the peerDeps missing.
I need to test this more before an eventual publish, but I was blocked by the fact that I can't publish beta versions, can someone help me with that?