Support Webpack 4.x #51
Conversation
package.json
Outdated
| @@ -25,6 +25,6 @@ | |||
| "devDependencies": { | |||
| "ava": "^0.22.0", | |||
| "changelog": "^1.4.0", | |||
| "webpack": "^3.6.0" | |||
| "webpack": "^4.5.0" | |||
There was a problem hiding this comment.
Could you update README.md to say now supports webpack 2-4 instead of 2-3?
test/Config.js
Outdated
| @@ -1,6 +1,6 @@ | |||
| import test from 'ava'; | |||
| import Config from '../src/Config'; | |||
| import { validate } from 'webpack'; | |||
| import { validateSchema } from 'webpack'; | |||
There was a problem hiding this comment.
I know #46 made this change, however I don't think it's correct. From what I can tell at first glance (and there aren't really any API docs), validateSchema expects both schema and options, so if called directly without a schema, it's not validating anything.
There was a problem hiding this comment.
Looks like you're right!
| @@ -0,0 +1,27 @@ | |||
| const ChainedMap = require('./ChainedMap'); | |||
|
|
|||
| module.exports = class extends ChainedMap { | |||
There was a problem hiding this comment.
Could you add a new test test/Optimization.js for this?
| @@ -347,6 +347,7 @@ config | |||
| .context(context) | |||
| .externals(externals) | |||
| .loader(loader) | |||
| .mode(mode) | |||
There was a problem hiding this comment.
The config merging section of the README also references the top level options, could you add mode there too? (I see parallelism is also missing from that, perhaps worth fixing whilst we're there)
Also, could you add the optimization options to the README too? :-)
|
Could you also rebase on master and resolve the conflict? I'd just delete |
src/Output.js
Outdated
| @@ -7,11 +7,14 @@ module.exports = class extends ChainedMap { | |||
| 'auxiliaryComment', | |||
| 'chunkFilename', | |||
| 'chunkLoadTimeout', | |||
| 'chunkCallbackName', | |||
There was a problem hiding this comment.
This is super nitpicky, but could you move this to be before chunkFilename so it stays in alphabetical order?
There was a problem hiding this comment.
There's a bit of a mix throughout webpack-chain at the moment of alphabetical order vs schema order - would be good to sort it all out at some point :-)
There was a problem hiding this comment.
Oh, I thought I already cleaned all those up... 🤔
There was a problem hiding this comment.
Ok, I did a very quick run through to get all of the extend() calls at least to specify in alphabetical order.
+1 to setting up linting and/or Prettier 😄
test/Config.js
Outdated
| @@ -85,14 +85,24 @@ test('toConfig with values', t => { | |||
| .output | |||
| .path('build') | |||
| .end() | |||
| .mode("development") | |||
There was a problem hiding this comment.
Can you replace all the double quotes in here with single quotes to match the style? I need to go in and add better linting tools some day. 😄
There was a problem hiding this comment.
Sure, no problem
|
Travis is failing on the node 6 job with: Turns out webpack 4 has bumped the minimum node version: ...could you increase the version specified in |
|
Thanks @edmorley & @eliperelman for reviewing so quickly! |
|
Our pleasure! Released in v4.6.0. |
Adds the `mode` and `optimizations` documentation from: neutrinojs/webpack-chain#51
Adds the `mode` and `optimizations` documentation from: neutrinojs/webpack-chain#51
Adds the `mode` and `optimizations` documentation from: neutrinojs/webpack-chain#51
Adds the `mode` and `optimizations` documentation from: neutrinojs/webpack-chain#51
Looks like PR #46 is pretty stale. I took a quick crack at updating to support webpack 4.x by going through the schema diff outlined in issue #45.
I'm currently trying to use
webpack-chainin a project, but not having support for webpack 4 is a pretty big blocker.