Skip to content

fix: add missing optional peer dependency webpack-cli #11189

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.

Sign up for GitHub

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

Jump to bottom
Merged
sokra merged 2 commits into from
Jul 20, 2020
Merged

fix: add missing optional peer dependency webpack-cli #11189

sokra merged 2 commits into from
Jul 20, 2020

Conversation

@merceyz
Copy link
Contributor

@merceyz merceyz commented Jul 17, 2020

Webpack tries to require webpack-cli without declaring it as a optional peer dependency which causes it to not get access to it under Yarn PnP

Fixes half of yarnpkg/berry#1452

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
No

Does this PR introduce a breaking change?
No

What needs to be documented once your changes are merged?
N/A

@jsf-clabot
Copy link

jsf-clabot commented Jul 17, 2020 •
edited
Loading

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@webpack-bot
Copy link
Contributor

webpack-bot commented Jul 17, 2020

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@alexander-akait
Copy link
Member

alexander-akait commented Jul 17, 2020 •
edited
Loading

Webpack doesn't use webpack-cli I am wrong

@merceyz
Copy link
Contributor Author

merceyz commented Jul 17, 2020 •
edited
Loading

It does actually, it tries to require it in these locations

webpack/bin/webpack.js

Line 36 in f4b5555

require.resolve(packageName);

webpack/bin/webpack.js

Line 119 in f4b5555

require(cli.package); //eslint-disable-line

webpack/bin/webpack.js

Line 128 in f4b5555

const pkgPath = require.resolve(`${cli.package}/package.json`);

alexander-akait
alexander-akait reviewed Jul 17, 2020
View reviewed changes
@merceyz merceyz mentioned this pull request Jul 17, 2020
Merged
@sokra
Copy link
Member

sokra commented Jul 17, 2020

Is it allowed to use peerDependenciesMeta without using peerDependencies?

@merceyz
Copy link
Contributor Author

merceyz commented Jul 17, 2020

Yes, Yarn 2 and potentially pnpm will treat it as if you also specified

"peerDependencies": {
  "webpack-cli": "*"
}

Yarn 1 and probably npm will just ignore it

@webpack-bot
Copy link
Contributor

webpack-bot commented Jul 17, 2020

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@alexander-akait
Copy link
Member

alexander-akait commented Jul 17, 2020

@sokra we use same for webpack-dev-server, https://github.com/webpack/webpack-dev-server/blob/master/package.json#L119

/cc @arcanis it is valid, right?

@arcanis
Copy link

arcanis commented Jul 17, 2020

Yep! 👍

@sokra sokra merged commit 6223b1c into webpack:master Jul 20, 2020
@sokra
Copy link
Member

sokra commented Jul 20, 2020

Thanks

@merceyz merceyz deleted the patch-1 branch July 20, 2020 10:31
@merceyz merceyz mentioned this pull request Jul 20, 2020
Closed
@merceyz merceyz mentioned this pull request Oct 19, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-akait alexander-akait alexander-akait left review comments

Assignees

No one assigned

Labels

PR: CI-ok PR: unreviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@merceyz @jsf-clabot @webpack-bot @alexander-akait @sokra @arcanis