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

fix: allow any eslint-config peer #65

Closed
wants to merge 1 commit into from

Conversation

lukekarrys
Copy link
Contributor

Ref: #64

@lukekarrys lukekarrys requested a review from a team as a code owner February 26, 2022 18:57
@wraithgar
Copy link
Member

My initial gut instinct is that this is a regression. However, if it's a peerDep it means that it's also in the package.json of whatever is using template-oss, and dependabot will be keeping THAT up to date. So, I think this is ok.

@lukekarrys
Copy link
Contributor Author

Maybe this is part of a larger discussion with #67, but I thought we were relying on peerDeps so that we didn’t need to have them in the main repo’s package.json.

here’s an example from make-fetch-happen, which doesn’t include @npmcli/eslint-config https://github.com/npm/make-fetch-happen/blob/main/package.json#L53-L62

@wraithgar
Copy link
Member

Oh right, peerDeps are installed by default now.

So in a dev's local environment, if they were on 2.0.0 of eslint-config they'd never update to 3 cause the 2 already satisfied it. In CI it would get 3 because * would resolve to latest by default. They could potentially get errors in CI they didn't get locally which is a little frustrating.

I do remember getting some peerdep warnings in older repos for situations like this, but they resolve after reification iirc. Is this an error that persists, or does it just happen once when a repo hasn't been touched in awhile?

@wraithgar
Copy link
Member

I think #67 is the real fix here. However we technically have the same issue w/ tap, if this is in fact an actual issue.

@lukekarrys lukekarrys closed this Mar 2, 2022
@lukekarrys lukekarrys deleted the lk/eslint-config-peer-range branch March 16, 2022 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants