Update webpack peerDependency in v1-stable branch to v4.x #265

mikegreiling · 2018-02-26T08:36:55Z

What kind of change does this PR introduce?

This adds webpack v4 as a devDependency, updates the file-loader dependency to fix tests, updates the config schema used in the server-tests, and adds webpack ^4.0.0 to supported peerDependencies.

Did you add tests for your changes?

No, but I fixed broken tests resulting from this change.

Summary

This pull request aims to add webpack v4 support to the 1.x versions of webpack-dev-middleware for projects which have not yet updated to 2.x.

See issue: #263

Does this PR introduce a breaking change?

No.

Other information

I have updated the webpack devDependency and the peerDependency config, and ensured that all tests pass. I have not, however, changed the version number in package.json for a v1.13 release.

Closes: #263
/cc @shellscape

mikegreiling · 2018-02-26T08:44:54Z

travis is failing here because node 4 is no longer supported by webpack. however I don't think this is an issue because those using webpack-dev-middleware can still use older versions of webpack that support older node versions. this isn't actually a breaking change, just an internally breaking change since the tests now target webpack v4.

shellscape · 2018-02-26T13:30:38Z

.travis.yml

@@ -1,7 +1,6 @@
 sudo: false
 language: node_js
 node_js:
-  - "4"


unfortunately we can't remove the Node v4 tests for v1.x of the module.

Webpack v4 does not support node 4. We could continue to keep the devDependency at webpack v3.x for the sake of these tests though.

All I really wanted to do is update the peerDependency to allow for v4.x.

Well in fairness, you can't remove a node dependency in a patch for major version that previously supported it. That's a massive semver violation, and that's why v2.x dropped support for it. Node 4 will have to stay and you'll have to find a way to not run webpack v4 tests on that node version. These are the things that bite you when trying to update an old version instead of upgrading.

We aren’t actually removing support for node 4 with this change. We just wouldn’t be running tests against it (because that is not possible with webpack v4). If we kept webpack v3 as the devDependency version we could keep running tests against node v4.

Really this situation isn’t dissimilar from what you’re already practicing. Webpack-dev-middleware technically supports webpack v1 and v2 but no tests are run for these versions... ¯_(ツ)_/¯

shellscape · 2018-02-26T13:31:13Z

test/Server.test.js

@@ -45,7 +45,7 @@ describe("Server", function() {
 		it("GET request to bundle file", function(done) {
 			request(app).get("/public/bundle.js")
 				.expect("Content-Type", "application/javascript; charset=UTF-8")
-				.expect("Content-Length", "2823")


is this change specific to webpack@4?

Yes, the Content-Length changed because webpack v4’s runtime code has changed between versions and now takes up a bit more space.

shellscape · 2018-02-26T13:31:17Z

test/Server.test.js

@@ -138,7 +138,7 @@ describe("Server", function() {

 		it("GET request to bundle file", function(done) {
 			request(app).get("/bundle.js")
-				.expect("Content-Length", "2823")


is this change specific to webpack@4?

same as above 👆🏻

shellscape · 2018-02-26T13:31:26Z

test/fixtures/server-test/webpack.config.js

 	context: __dirname,
 	entry: "./foo.js",
 	output: {
 		filename: "bundle.js",
 		path: "/"
 	},
 	module: {
-		loaders: [
+		rules: [


is this change specific to webpack@4?

Actually this schema name change started in webpack v2 (loaders was deprecated in favor of rules), but it wasn’t removed until v4.

OK this change can stick around

mikegreiling · 2018-02-26T21:59:30Z

@shellscape here’s my thoughts. Unfortunately while webpack-dev-middleware appears to be compatible with webpack v2 through v4, we can only run our tests against one version of webpack at a time because we can only specify a single version in our devDependencies. As I presume most people using the legacy branch will be using webpack v2 or v3, perhaps we ought to keep webpack v3 as the test target.

However there is no reason we cannot add v4.x as a valid peerDependency. Webpack v4 clearly does work with webpack-dev-middleware v1.12 as this PR demonstrates. All of the changes I made were to the test fixtures, not to the code of the library itself.

Let me know how you’d like me to proceed.

shellscape · 2018-02-26T22:05:47Z

You're going to have to do a few things here to get this pushed through:

All tests on Travis CI must run and succeed on both webpack v3 and webpack v4.
Values which are different between webpack versions will have to be intelligently asserted in tests.
Tests for webpack v4 cannot run under Node v4

To get the webpack version:

const webpackPackage = require('webpack/package.json');
const webpackVersion = parseInt(webpackPackage.version, 10);

I've never setup a complex travis config like what's needed here, but I do have several modules running against webpack v3 and v4. Since webpack@3 is the default devDep, .travis.yml will need to look something like this:

script:
  - npm run ci                  # run linting and testing, generate coverage
  - npm install webpack@latest  # install webpack@4
  - npm run mocha               # run mocha tests against webpack@3, without coverage

It's a lot, but we can't just shoehorn support in without being absolutely sure that we're not breaking a (past) major version. The one thing not covered here is why you haven't upgraded to v2 and called it a day.

mikegreiling · 2018-02-26T22:11:12Z

I can do this if it is what you require, however as I mentioned earlier it doesn’t look like this approach was taken for support of webpack v1 and v2. Both of these are supported as peerDependencies and there are no tests being run with these versions.

shellscape · 2018-02-26T22:15:03Z

I wasn't a maintainer for the versions that covered webpack v1 an v2 😄 and no version has been as disruptive as webpack v4 to date. Releasing new major versions is how we support the new majors of the primary dependencies. Is it worth the time to do all of this, or simply upgrade to webpack-dev-middleware v2.x?

If you cannot (or are unwilling) to upgrade your version of webpack-dev-middleware, you may want to consider forking, which won't require the same hoops to jump through. I'd be happy to link to your fork for people who can't/won't move past 1.x and need that peerDep setting changed.

mikegreiling · 2018-02-26T22:19:50Z

I suppose it probably would have been faster to upgrade karma-webpack to use v2.x, but this seemed like a simpler approach as I already knew it worked with v1 and just needed a peerDependency change in order to fix the warnings I was seeing. I assumed the same was likely true for other libraries which haven’t upgraded yet so I figured fixing something once here would be less effort than fixing something in several other places.

Since I’ve already done half the work, I’ll see if I can just finish the job here and add support for v3 and v4 in the travis config. I presume this change would also be welcome in the v2 branch since it appears you wish to continue supporting webpack v3 there as well.

mikegreiling · 2018-02-26T22:23:25Z

I would like to note also that the current version of webpack-dev-middleware v2 you’ve released supports webpack v4 as a peerDependency without any tests (see #251 )... Was that a mistake? I’m not sure why this wasn’t worthy of the same exception.

shellscape · 2018-02-26T22:43:58Z

I don't know how much you followed the webpack v4 alpha/beta/release cycle, but it was impossible to keep up with. Hence no formal suite of tests covering it, yet. I'm presently going through that pain on webpack-serve and webpack-hot-client so I'm also not sure if that was a dig at me about dev-middleware not being up to par yet... but I'm only one human, cranking out free code in the free time that I can manage.

If I have my way, every new major will have tests running in CI to cover the last two versions of webpack. That is until we just start mandating that new major versions of companion modules only support the current version. Be glad that someone is making this push so folks a version back on webpack don't get left in the dust.

mikegreiling · 2018-02-26T22:54:38Z

That was not meant as a dig at you personally. I appreciate you taking the time to be thorough. 😅 I was just pointing out what appeared to me to be a slight double standard – one that you may not have been conscious of.

I’ll update this PR as soon as I can find the time, assuming you’re still ok with merging it. I’ll update #266 as well.

shellscape · 2018-02-26T23:27:26Z

Yessir, I'm still OK to merge it as long as we have our bases covered.

FWIW in this ecosystem (webpack) it's not so much a double standard as it is misalignment or inconsistency. Much of the time it's just simply a lack of time to do all the things needed to maintain that alignment.

shellscape · 2018-03-20T16:23:23Z

@mikegreiling is this still a need for your team?

mikegreiling · 2018-03-20T19:06:50Z

@shellscape I had intended to return to this but got I busy with other things. In the mean time it looks like karma-webpack has updated its webpack-dev-middleware dependency with codymikol/karma-webpack#318

I’m going to close this now as it is no longer needed for me. Thanks for being willing to work with me on it 👍

update webpack dependency to v4.x

0ed31b1

mikegreiling mentioned this pull request Feb 26, 2018

Release v1.13 with webpack v4 listed as peedDependency #263

Closed

2 tasks

remove node 4 tests from travis

0700c9e

mikegreiling mentioned this pull request Feb 26, 2018

Update webpack devDependency to run tests against v4 #266

Closed

mikegreiling changed the title ~~Update webpack dependency in v1-stable branch to v4.x~~ Update webpack peerDependency in v1-stable branch to v4.x Feb 26, 2018

shellscape suggested changes Feb 26, 2018

View reviewed changes

mikegreiling closed this Mar 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update webpack peerDependency in v1-stable branch to v4.x #265

Update webpack peerDependency in v1-stable branch to v4.x #265

mikegreiling commented Feb 26, 2018 •

edited

Loading

mikegreiling commented Feb 26, 2018

shellscape Feb 26, 2018

mikegreiling Feb 26, 2018

shellscape Feb 26, 2018

mikegreiling Feb 26, 2018

shellscape Feb 26, 2018

mikegreiling Feb 26, 2018

shellscape Feb 26, 2018

mikegreiling Feb 26, 2018

shellscape Feb 26, 2018

mikegreiling Feb 26, 2018

shellscape Feb 26, 2018

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018 •

edited

Loading

mikegreiling commented Feb 26, 2018

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018 •

edited

Loading

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018

shellscape commented Mar 20, 2018

mikegreiling commented Mar 20, 2018

Update webpack peerDependency in v1-stable branch to v4.x #265

Update webpack peerDependency in v1-stable branch to v4.x #265

Conversation

mikegreiling commented Feb 26, 2018 • edited Loading

mikegreiling commented Feb 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018 • edited Loading

mikegreiling commented Feb 26, 2018

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018 • edited Loading

mikegreiling commented Feb 26, 2018

shellscape commented Feb 26, 2018

shellscape commented Mar 20, 2018

mikegreiling commented Mar 20, 2018

mikegreiling commented Feb 26, 2018 •

edited

Loading

shellscape commented Feb 26, 2018 •

edited

Loading

shellscape commented Feb 26, 2018 •

edited

Loading