Webpack 4 (only) compatibility #123
Conversation
|
Oops didn't see this...When I made #125...when will the maintainers merge this? Or should we fork and create a new npm module? |
|
Also, is there any reason we wouldn't want to support webpack 1, 2, 3, since the functionality is already there? |
|
So, not meaning to steal yall's thunder or anything, but I went ahead and published this PR, because I need it for a project and don't want to install it from github....but I will redirect to this library as soon as it is published, or if you guys want to take over my npm package, that would be cool, because the name is outdated, because it supports yarn now too, although, I guess you could still say that the dependencies are ultimately still published to npm too Anyway, the npm package is https://npmjs.com/webpack-plugin-install-deps, and anyone else can use it as well...I'll try not to make any breaking changes and still keep it up to date until this is merged. |
| @@ -38,21 +38,27 @@ function NpmInstallPlugin(options) { | |||
| NpmInstallPlugin.prototype.apply = function(compiler) { | |||
| this.compiler = compiler; | |||
|
|
|||
| var plugin = { name: "NpmInstallPlugin" }; | |||
|
|
|||
| // Recursively install missing dependencies so primary build doesn't fail | |||
There was a problem hiding this comment.
@insin Can you add a check here for webpack < 4 like this
if (compiler.hooks && compiler.hooks.watchRun ) { // all other hooks being used
// webpack 4
}else{
// webpack 3
}Thanks!
also Is this PR gonna merge ?!?
There was a problem hiding this comment.
I'm also wondering what is preventing this from being updated. I see it was failing a test and without diving in it looks like the "before each hook" should "checkBabel" error could be fixed or worked around easily.
At this point this is a webpack-contrib plugin referred to in official docs but is incompatible with the current version of Webpack. If there is a reason the PR solution can't be adapted and merged, I think @insin should publish this in its own repo.
The process of trying to find a plugin that did this using v4 was pretty confusing. I searched npm and eventually found the forked version. It's readme was the same as this repos, aside from mentioning that it was an unmerged PR for v4 support at the top. The repo link in that readme goes to this repo which is pretty much dead at this point.
So, If this isn't going to be merged, why? And if not, the webpack-plugin-install-deps should have it's own repo and updated readme indicating 4 support. Either way, it still makes more since for this being a webpack-contrib repo instead of leaving this obsolete one up and not having a solution for v4 and since @insin just wanted to contribute and indicated he would prefer not to maintain a separate repo, especially since this repo would just fade into limbo without current support.
|
/cc @anikethsaha need fix test in next PR |
Ok. will submit a follow-up test fixes soon 👍 |
|
@evilebottnawi Do you have an ETA on publishing? Would love to get this reintegrated with my project now that this has been merged. Thank you for working on it again! |
|
Although it's safe to release now. But currently, tests are failing because tests are for the old version and its not compatible for v4 + |
|
@anikethsaha @mAAdhaTTah yes we need fix tests before release |
I'm wondering if the note about
Compiler.optionsin the Webpack 4 release notes only applies to loaders.PR for #122
Basic functionality working for me in the Webpack 4 version of nwb: