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

Adds an optional peer dependency on webpack-cli #2396

Merged
merged 1 commit into from
Jan 28, 2020

Conversation

arcanis
Copy link
Contributor

@arcanis arcanis commented Jan 18, 2020

  • This is a bugfix
  • This is a metadata update

For Bugs and Features; did you add new tests?

Can publish a package with this PR and see if it actually works but these kind of fixes have been done multiple times to make Yarn 2 happy. Considering webpack-dev-server throws at runtime anyway if webpack-cli is missing this shouldn't bother anyone

Motivation / Use-Case

Yarn 2 currently throws when using webpack-dev-server due to it trying to require webpack-cli without declaring it anywhere in its dependencies:

Error: A package is trying to access another package without the second one being listed as a dependency of the first one

Required package: webpack-cli (via "webpack-cli/bin/config-yargs")
Required by: webpack-dev-server@virtual:836be30d0802b4899b9a78ca9d744f43f038cccf96d6b8307cbd938d17151f1ac108d2e64290515733c51d0949e0c6d87f9a7c9e245dfe628e5c4ef98f22d752#npm:3.8.0 

Breaking Changes

There is no breaking change for anyone - not even a warning for older package managers. This is a completely safe bugfix that will only impact Yarn users.

Additional Info

Supercedes: #2191

Documentation: https://next.yarnpkg.com/configuration/manifest#peerDependenciesMeta.optional

@codecov
Copy link

codecov bot commented Jan 18, 2020

Codecov Report

Merging #2396 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2396   +/-   ##
=======================================
  Coverage   93.75%   93.75%           
=======================================
  Files          34       34           
  Lines        1297     1297           
  Branches      370      370           
=======================================
  Hits         1216     1216           
  Misses         79       79           
  Partials        2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9937f5...c482546. Read the comment docs.

@arcanis
Copy link
Contributor Author

arcanis commented Jan 22, 2020

Ping @evilebottnawi?

@alexander-akait
Copy link
Member

@arcanis npm still not fixed bug about warning optional peer deps?

@arcanis
Copy link
Contributor Author

arcanis commented Jan 22, 2020

They have fixed in November, so if you want to merge the other PR it'll be fine. If you're still a bit wary that's ok, that's why I opened this alternate PR: it only adds the peerDependenciesMeta field (without explicit peerDependency), so it won't affect npm at all. It's a good in-between.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Sounds good!

/cc @hiroppy @Loonride

After merge i will do release

@alexander-akait
Copy link
Member

/cc @hiroppy @Loonride

@alexander-akait
Copy link
Member

@hiroppy @Loonride feel free to feedback

@alexander-akait alexander-akait merged commit aa365df into webpack:master Jan 28, 2020
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.

4 participants