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

Fresh install complains "webpack-dev-server@3.9.0" has unmet peer dependency "webpack@^4.0.0"." #2371

Closed
jrochkind opened this issue Nov 19, 2019 · 8 comments

Comments

@jrochkind
Copy link

jrochkind commented Nov 19, 2019

Create a fresh install with Rails 6.0.1, by just doing rails new myapp, which will cause webpacker:install task to be run.

Rails 6.0.1, webpacker 4.2.0, webpack-dev-server 3.9.0, webpack 4.41.2, yarn 1.19.1.

In the output of yarn install, you can see:

warning " > webpack-dev-server@3.9.0" has unmet peer dependency "webpack@^4.0.0".
warning "webpack-dev-server > webpack-dev-middleware@3.7.2" has unmet peer dependency "webpack@^4.0.0".

Expected

I expect a freshly created project created with rails new and/or rake webpacker:install, which has not been customized at all, should not produce any warnings.

Fix?

Am I expected to add a "webpack" dependency to my package.json myself, is this warning telling me it should be there? Maybe it should just be a devDependency since that's what webpack-dev-server is? I am not sure what the right thing to do is, or if it will have any unintended side-effects, which is why the generator should do it for me instead of leaving me with a warning.

Should webpacker:install templates/generators add a webpack dependency to package.json to begin with, to go along with the webpack-dev-server one added already?

@jrochkind
Copy link
Author

Some more context (I'm new to understanding npm dependency resolution, so figuring this out as a go along).

In a freshly generated app, you have webpack there because it is a dependency of webpacker. Not a "peer dependency", just a straight dependency. In webpacker 4.2.0, specifically to webpack ^4.41.2.

Meanwhile webpack-dev-server 3.9.0 does not have a direct dependency to webpack, but instead a "peer" dependency to ^4.0.0.

And the resolution mechanism apparently doesn't recognize the indirect dependency from webpacker as fulfilling that peer dependency, so complains it's missing. But, even though webpack-dev-server can't actually work without webpack... it does work, cause webpack is there, as an indirect dependency of webpacker.

I am not sure if this is a bug in how npm resolves peer dependencies maybe it ought to be recognizing the indirect dependency as satisfying the peer dependency? I don't fully have my head around the intents and consequences here.

If you manually add "webpack": "^4.0.0" to your app package.json, it does make npm satisfied that webpack-dev-server's peer dependency is met, and it stops complaining. It doesn't appear to have any other effects on your dependency tree -- the yarn.lock is only changed to note that this dependency requirement exists, but the actual resolutions into packages isn't changed at all, you still have the same single webpack resolution in your tree.

But, what if some point in the future there's a new release of webpacker that depends on webpack ^5.0.0 instead? If you upgrade to that, but forget to change your direct webpack dependency in your app package.json.... this is going to result in you having two webpacks in your dependency tree I think, and be generally confusing?

So what should be done?

  1. It's a bug in npm's dependency resolution, nothing should be done, user's are just going to get the warning. (In which case docs mentioning the warning is expected and you shouldn't do anything would help; this question comes up a lot, because people rightly don't want to let warnings they don't understand cry wolf!)

  2. Devs should just add the direct webpack dependency to their package.json. In which case, the webpacker:install task should probably do it for them, why make them do it manually, isn't this the point of the installer?

  3. Maybe webpacker itself should only have a peer dependency to webpack not a direct ordinary dependency? Then combined with 2 above, installer should put the webpack dependency in app package.json, now we avoid the possibility of accidentally having two webpacks in the dependency tree.

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Nov 20, 2019

  1. It is not a bug, webpacker makes use of unadvised node_modules behavior.
    2 & 3. I remember there being a PR for each of these.

There are many issues that discuss this at length, here is a recent one: #2191 (comment)

These issues will need to be fixed for Yarn Plug’n’Play: #2112

@jrochkind
Copy link
Author

OK, thanks for feedback and providing references to see more.

It's been very hard for me starting from not much experience with yarn/npm/node_modules to figure out what's going on, what who considers a bug or problem, to find the existing discussion on it. This helps, and hopefully will be helpful to others.

I am not sure what you mean by "unadvised node_modules behavior", I don't understand that phrase. Especially in relation to it being a bug or not -- is it a bug in webpacker that it uses "unadvised node_modules behavior"?

Having the webpacker:install add a direct webpack dependency to package.json is a pretty straightforward PR. Likewise having webpacker only have a peer dependency and not a direct dependency on webpack.

The hard part for me is figuring out what the right thing to do is. And getting everyone on the same page with that. But if there's consensus those would be right (?) -- they are pretty straightforward PRs -- should I just make em? Or am I missing something about barriers to proceeding with those?

@jakeNiemiec
Copy link
Member

I am not sure what you mean by "unadvised node_modules behavior"

You know when you see a sign that says "not a step"? This is the equivalent that. In short: Don't depend on this, it is not intended to be used this way.

The hard part for me is figuring out what the right thing to do is. And getting everyone on the same page with that. But if there's consensus those would be right (?) -- they are pretty straightforward PRs -- should I just make em? Or am I missing something about barriers to proceeding with those?

My advice is the same as when you brought this up 7 months ago in #1078 (comment). We can continue this thread there if you wish.

@jrochkind
Copy link
Author

OK, the thing is, I don't understand what's going on enough to be sure it is a good idea to add webpack to local package.json.

With it as a root dependnecy in package.json and a dependency of webpacker... from what I understand of things, that introduces the possibility to have two different versions of webpack in your node_modules, with potentially confusing and disastrous consequences.

But am I wrong, this is not really a problem, it's okay just to add it as a local root dep in package.json , despite it also being a dependency of webpacker?

It's easy to enough to construct the PR, the hard part is knowing the right thing to do here. As I look more and more into it.. I think more and more that javascript dependency management is pretty hairy and I don't understand it! Which makes me reluctant to just send a PR that I can't myself be confident wont' cause other problems down the line. But if you think that's the right thing to do?

Unlike 7 months ago, with most recent version of webpacker, there is only this one warning about unmet peer dependency with a freshly generated app, only for webpack.

@jakeNiemiec
Copy link
Member

There is already a problem coming down the line. Both npm (tink) & yarn (pnp) are moving to a pnp model.
I point you again to #2112
image

Which makes me reluctant to just send a PR that I can't myself be confident wont' cause other problems down the line.

I linked the file that you would need to change in the other thread. It would be useful to discuss this in the context of a PR instead of hypotheticals here.

@jrochkind
Copy link
Author

jrochkind commented Nov 21, 2019

OK. Yep, I've been reading about yarn pnp and it's issues here, after you mentioned it, although I'm not familiar with tink.

I am pretty sure including something as a peer dependency allows you to require it, at least for yarn pnp.

I'm still not sure what's what, but I can try to prepare a PR if that makes it easier to discuss.

I may switch webpack to a peer dependency in the PR, for the sake of discussion, as I think that may be the correct answer. (As on the model that webpack is a peer dependency of webpack-dev-server, not an ordinary dependency).

@jakeNiemiec
Copy link
Member

Quite a few of these should be peer dependencies: #2191 (comment)

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

No branches or pull requests

2 participants