Skip to content

chore: declare TypeScript as an optional peerDependency #990

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

Merged
merged 6 commits into from
Oct 14, 2019

Conversation

dstaley
Copy link
Contributor

@dstaley dstaley commented Sep 19, 2019

This PR adds TypeScript as an optional peer dependency for @typescript-eslint/eslint-plugin. This modification is necessary to get @typescript-eslint/eslint-plugin working with Yarn v2, and is one of two packages that need this modification so that the TypeScript version of create-react-app can work with Yarn v2.

Optional peer dependencies work as expected in Yarn and pnpm, and will be supported in npm once a server-side fix is deployed.

Other relevant issues: yarnpkg/berry/pull/475

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @dstaley!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@bradzacher bradzacher added the dependencies Issue about dependencies of the package label Oct 10, 2019
@bradzacher bradzacher changed the title Declare TypeScript as an optional peerDependency code: declare TypeScript as an optional peerDependency Oct 10, 2019
@bradzacher
Copy link
Member

Thanks for opening this.
This should also be done for typescript-estree.

We were planning on doing this ourselves, but we wanted to wait until the fix landed and was well rolled out within npm.

@bradzacher bradzacher changed the title code: declare TypeScript as an optional peerDependency chore: declare TypeScript as an optional peerDependency Oct 10, 2019
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

rc

@dstaley dstaley requested a review from bradzacher October 10, 2019 19:50
@arcanis
Copy link
Contributor

arcanis commented Oct 14, 2019

@dstaley @bradzacher Can you at least add the peerDependenciesMeta without peerDependencies for now? Yarn will now interpret this pattern as an implicit peer dependency on *, and npm won't print warnings about it.

@bradzacher
Copy link
Member

bradzacher commented Oct 14, 2019

That's awesome! I didn't know that it did that.
I think we can definitely add the meta by itself without delaying.
Happy to accept a PR, or I'll raise one tonight.

or alternately, happy if @dstaley modifies this PR to remove the explicit peerDependencies so we can merge it now.

@dstaley
Copy link
Contributor Author

dstaley commented Oct 14, 2019

@bradzacher explicit dependencies have been removed!

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

thanks for this!

@arcanis
Copy link
Contributor

arcanis commented Oct 14, 2019

Awesome, thank you both! 🥳

@bradzacher bradzacher merged commit 1918024 into typescript-eslint:master Oct 14, 2019
@bradzacher
Copy link
Member

FYI - we missed the release by a bit, so this will officially release next monday.
But it will release on the canary tag within a few minutes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Issue about dependencies of the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants