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: add missing peer dependencies #507

Merged
merged 6 commits into from Oct 6, 2021

Conversation

merceyz
Copy link
Contributor

@merceyz merceyz commented Aug 30, 2020

What's the problem this PR addresses?

fork-ts-checker-webpack-plugin depends on some peer dependencies without declaring them as such, this makes it rely on hoisting to place the dependencies in an accessible location which is not guaranteed to happen.

How did you fix it?

Declare typescript and webpack as peer dependencies and vue-template-compiler and eslint as an optional peer dependency

@codeclimate
Copy link

codeclimate bot commented Sep 10, 2020

Code Climate has analyzed commit 152cc22 and detected 0 issues on this pull request.

View more on Code Climate.

@slavafomin
Copy link

@piotr-oles could you please look into this issue? I've stumbled upon the fact that ForkTsCheckerWebpackPlugin isn't listing it's peer dependencies correctly. This leads to some serious problems when incorrect version of webpack is getting resolved by package manager. Which in turn leads to typing errors (i.e. webpack ≠ webpack).

@slavafomin
Copy link

pnpm workaround

If you are using pnpm, you can temporarily fix this problem using the following pnpmfile.js:

module.exports = {
  hooks: { readPackage },
};

function readPackage($package, context) {
  if ($package.name === 'fork-ts-checker-webpack-plugin') {
    $package.peerDependencies.webpack = '^5.0.0';
  }
  return $package;
}

@stale
Copy link

stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 26, 2021
@stale stale bot removed the wontfix label Jun 27, 2021
@Methuselah96
Copy link
Contributor

Methuselah96 commented Oct 5, 2021

@piotr-oles What's status of this PR? The latest comment was that it would produce unwanted warnings, however this is not the case since all major package managers support peerDependenciesMeta and these peer dependencies would be marked as optional.

I would be happy to create a new PR that specifically targets alpha so we get the added benefit of releasing it in a major version in case it causes issues.

@piotr-oles
Copy link
Collaborator

I think we can merge this to the main branch if it's well supported :)

@piotr-oles piotr-oles enabled auto-merge (squash) October 6, 2021 07:43
@Methuselah96
Copy link
Contributor

@piotr-oles Awesome! Looks like it just needs to be approved.

@piotr-oles piotr-oles merged commit 34ebcd8 into TypeStrong:main Oct 6, 2021
Methuselah96 added a commit to Methuselah96/fork-ts-checker-webpack-plugin that referenced this pull request Oct 6, 2021
This is a followup to TypeStrong#507. I'm guessing CRA would want all of the peer dependencies optional not just some of them. I could be mistaken, but I don't want the previous PR to cause any problems for CRA.
@piotr-oles
Copy link
Collaborator

🎉 This PR is included in version 6.3.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@merceyz merceyz deleted the missing-peer-dep branch October 6, 2021 13:43
Methuselah96 added a commit to Methuselah96/berry that referenced this pull request Oct 6, 2021
@piotr-oles
Copy link
Collaborator

🎉 This PR is included in version 7.0.0-alpha.15 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants