webpack-contrib / install-webpack-plugin

Speed up development by automatically installing & saving dependencies with Webpack.
MIT License
1.43k stars 92 forks source link

Tries to install html-webpack-plugin-loader if ^2.21.0 #91

Closed bebraw closed 7 years ago

bebraw commented 7 years ago

If html-webpack-plugin ^2.21.0 is installed (tested with 2.26.0), npm-install-webpack-plugin tries to install it as a loader. Interestingly 2.21.0 works just fine. A release after that broke the behavior as it gets detected improperly.

To make this easier to debug, I set up a repository. npm install and npm start to get

Installing html-webpack-plugin-loader...
npm ERR! Darwin 16.3.0
npm ERR! argv "/usr/local/Cellar/node6-lts/6.9.1/bin/node" "/usr/local/bin/npm" "install" "html-webpack-plugin-loader" "--save"
npm ERR! node v6.9.1
npm ERR! npm  v3.10.8
npm ERR! code E404

npm ERR! 404 Registry returned 404 for GET on https://registry.npmjs.org/html-webpack-plugin-loader
npm ERR! 404
npm ERR! 404  'html-webpack-plugin-loader' is not in the npm registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
npm ERR! 404
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.
bebraw commented 7 years ago

To be more accurate, it looks like html-webpack-plugin 2.25.0 is the first broken version. So something changed in between 2.24.0 and 2.25.0 that broke the behavior.

bebraw commented 7 years ago

Here is a diff between the versions. I hope that helps.

bebraw commented 7 years ago

I see now. html-webpack-plugin has a line like this now:

template = 'html-webpack-plugin/lib/loader.js!' + path.resolve(context, template);

So it actually has a loader inside. That's why npm-install-webpack-plugin logic triggers the way it does I think.

I wonder if there's a good way to detect this type of case.

bebraw commented 7 years ago

Got it. resolveLoader gets result.request like this html-webpack-plugin/lib/loader.js.

I would suggest checking whether or not that file exists in the file system (fs.exists or so against node_modules). If it does, then installing can be skipped.

insin commented 7 years ago

I'm also seeing this with a fallback in place to account for what html-webpack-plugin is now doing.

    // As of v2.25.0, html-webpack-plugin no longer outputs an absolute path to
    // its loader, so we must fall back to nwb's node_modules/ for global usage.
    resolveLoader: {
      fallback: path.join(__dirname, '../node_modules'),
    },

Will investigate further.

insin commented 7 years ago

Making NpmInstallPlugin#resolveLoader() behave more like NpmInstallPlugin#resolveModule() fixes this for me:

I'm assuming my change gives Webpack a shot at resolving the loader first, allowing it to use your resolveLoader config?

Does this seem correct to someone more familiar with what Webpack is doing? I can fix the tests and create a PR if so.

bebraw commented 7 years ago

@ericclemmons How do you want to move with this? Should I prepare a little PR with the check outlined above? Any better ideas? Also what @insin mentioned could be worthwhile.

ericclemmons commented 7 years ago

@bebraw @insin's fix seems to be the best way to resolve it, IMO:

https://github.com/insin/npm-install-webpack-plugin/commit/c748308fdc3eb166b571d2a1bcf55d6343d5de15

With the way things have been going with webpack2, loader & module resolution should work largely the same...

bebraw commented 7 years ago

@ericclemmons Awesome.