chore(deps): add webpack-cli to peers

[eps1lon]

Follow-up on #2188 according to #2188 (comment)

• This is a bugfix
• This is a feature
• This is a code refactor
• This is a test update
• This is a docs update
• 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 v2 happy. Considering webpack-dev-server throws at runtime anyway if webpack-cli is missing this shouldn't bother anyone

Motivation / Use-Case

yarn v2 throws currently with

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

I guess this new warning might be annoying if you don't run the webpack-dev-server cli. If this is a blocker I can try and see if https://github.com/yarnpkg/rfcs/blob/master/accepted/0000-optional-peer-dependencies.md works for yarn v2.

Additional Info

https://github.com/yarnpkg/rfcs/blob/master/accepted/0000-optional-peer-dependencies.md

[alexander-akait]

Please remove webpack-cli from here, this line break CFA, vue-cli, angular-cli and etc big boilerplates

[alexander-akait]

/cc @arcanis it is require what webpack-cli was in peerDependencies? Because we can't do this right now

[codecov]

Codecov Report

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

@@           Coverage Diff           @@
##           master    #2191   +/-   ##
=======================================
  Coverage   93.91%   93.91%           
=======================================
  Files          34       34           
  Lines        1282     1282           
  Branches      370      370           
=======================================
  Hits         1204     1204           
  Misses         71       71           
  Partials        7        7

Impacted Files	Coverage Δ
lib/Server.js	97.33% <ø> (ø)	⬆️

---

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 8a112ff...f1b8598. Read the comment docs.

[arcanis]

/cc @arcanis it is require what webpack-cli was in peerDependencies?

Yes. The peerDependenciesMeta field affect how package managers will treat the entries for the existing peerDependencies field - it doesn't extend it.

Because we can't do this right now

Again, if you can't only because it would cause warnings to appear for npm's users then I'd say it's too bad because omitting the peer dependencies will cause both npm and Yarn to silently generate invalid dependency trees, whereas warnings will only cause a slight temporary discomfort at worst.

Does it actually break something? As in, a consumer stops working altogether?

[alexander-akait]

@arcanis do you know other developers, they starts create a lot of issues and send PRs with removing this, we want remove bin for webpack-dev-server@4 so i think better avoid this right now for webpack-dev-server@3

[arcanis]

If this is a blocker I can try and see if https://github.com/yarnpkg/rfcs/blob/master/accepted/0000-optional-peer-dependencies.md works for yarn v2.

Optional peer dependencies work for Yarn 1, Yarn 2, pnpm 3.2+, and (planned for) npm 6.11+.

you know other developers, they starts create a lot of issues and send PRs with removing this

I sympathise, however note that the same is also true for us - we get issues telling us that Yarn is installing dependencies the wrong way etc, and after digging we notice it's caused by undeclared dependencies like this. Plus I guess you still get PRs to add it, in the end 😄

[alexander-akait]

@arcanis but here not Optional peer dependencies

[arcanis]

They are; that's how optional peer dependencies work. It's not a optionalPeerDependencies field.

Instead, the peerDependenciesMeta only lists settings that modify the default behaviour for the matching peer dependency field of the given name (optional means that the peer dependency being unmet shouldn't be reported as a warning, for example).

The peer dependencies still have to be declared in peerDependencies (otherwise we wouldn't have the range hint for the peer dependency, for example).

[alexander-akait]

@arcanis thansk for clarify, maybe we should wait npm/cli#224 before merge and release this

[eps1lon]

@evilebottnawi npm@6.11 was released which includes npm/cli#224

I don't know how to make the checks pass though. Failures seem unrelated.

[hiroppy]

We need to wait for npm to be updated in Node.js.

[salper]

We need to wait for npm to be updated in Node.js.

It seems to be the case. Any news on this?

[arcanis]

@evilebottnawi @hiroppy I've opened another PR (#2396) that uses a smoother pattern and doesn't print any warnings even on older package managers. Can one of you give it a look and tell me how it looks? We plan to release Yarn 2 next week, it would be nice to sort this out before then if possible 😀

[alexander-akait]

Fixed for yarn v2, feel free to feedback