chore: upgrade dev middleware #2660

knagaitsev · 2020-06-25T07:08:57Z

For Bugs and Features; did you add new tests?

Not yet

Motivation / Use-Case

webpack-dev-middleware needs to be updated

Take a look at the options.json file and tell me if these changes look good for the options.

Breaking Changes

Many options being removed in favor of devMiddleware option.

Change where publicPath is handled, which is minor breaking change for API

Additional Info

codecov · 2020-06-25T07:20:31Z

Codecov Report

Merging #2660 into v4 will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##               v4    #2660      +/-   ##
==========================================
+ Coverage   92.41%   92.53%   +0.11%     
==========================================
  Files          34       37       +3     
  Lines        1292     1312      +20     
  Branches      357      355       -2     
==========================================
+ Hits         1194     1214      +20     
  Misses         93       93              
  Partials        5        5

Impacted Files	Coverage Δ
lib/Server.js	`95.98% <100.00%> (+0.07%)`	⬆️
lib/utils/createConfig.js	`95.18% <100.00%> (-0.33%)`	⬇️
lib/utils/getColorsOption.js	`100.00% <100.00%> (ø)`
lib/utils/getCompilerConfigArray.js	`100.00% <100.00%> (ø)`
lib/utils/getStatsOption.js	`100.00% <100.00%> (ø)`
lib/utils/normalizeOptions.js	`100.00% <100.00%> (ø)`
lib/utils/routes.js	`92.68% <100.00%> (+0.37%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb9c58d...30b3443. Read the comment docs.

alexander-akait · 2020-06-25T12:02:47Z

lib/options.json

-      "type": "boolean"
-    },
-    "fs": {
+    "devMiddleware": {


It is good place to do discussion, should we move all webpack-dev-middleware in the one place

/cc @hiroppy

I think it makes sense because webpack-dev-middleware now does options validation that only allows for the properties specified. So we can just validate devMiddleware as an object, and if the user passes in bad webpack-dev-middleware options then webpack-dev-middleware validation will throw an error.

I prefer this idea.

Let's rename it to dev, I think it is unnecessary to have middleware suffix, we don't have it for other middlewares

Also, it will be better for CLI - webpack server --dev-something=value

knagaitsev · 2020-06-26T04:35:24Z

test/server/Server.test.js

        const compiler = webpack(config);
        const server = new Server(compiler, baseDevConfig);

        server.invalidate();
-        expect(server.middleware.context.callbacks[0]).toBe(noop);
+        expect(server.middleware.context.callbacks.length).toEqual(1);


webpack-dev-middleware no longer exports the noop function. Should we make it get exported from webpack-dev-middleware, or do you think this is sufficient for this test?

I think it's ok.

alexander-akait · 2020-06-26T13:34:28Z

We have broken tests, maybe it is regressions, we need collect them here

knagaitsev · 2020-06-26T18:04:10Z

@evilebottnawi Yes, lots of broken tests because of the options changes that I still need to fix, I just wanted to confirm that you agreed with switching to devMiddleware option name

knagaitsev · 2020-06-28T16:51:19Z

lib/utils/normalizeOptions.js

+
+  // this was previously done in createConfig, but it should be done
+  // here for CLI/API uniformity
+  if (!options.devMiddleware.publicPath) {


I thought this was a good time to move publicPath handling from createConfig into normalizeOptions so it is done both on the CLI and API. Let me know if you disagree, as it is a minor breaking change for API.

EDIT: changed my mind, this can happen in a different PR.

Yes, let's think about it in other PRs

alexander-akait · 2020-06-29T12:02:33Z

@Loonride Agree, let's move options to the dev object

alexander-akait · 2020-06-29T12:04:53Z

I think we should remove stats option from webpack-dev-server because new version of webpack-dev-middleware take value from configuration, so you don't need it anymore, if you want to change it you can use stats: WEBPACK_DEV_SERVER ? 'warnings-errors' : 'verbose'

knagaitsev · 2020-06-29T22:32:06Z

All tests should be passing once webpack/webpack-dev-middleware#670 is released, then I update the webpack-dev-middleware dependency in this PR.

knagaitsev · 2020-06-29T22:40:21Z

bin/options.js

@@ -62,17 +62,6 @@ const options = {
    describe: 'Open default browser with the specified page',
    requiresArg: true,
  },
-  color: {


I removed the color CLI flag since it ends up setting colors: true in the stats option, which was also removed.

New behavior:

default for colors is false (should it be true?)

if a user wants to turn on color, they need to put in their webpack config: stats: { colors: true }.

Do you think we should do this approach, or should we keep the color CLI flag and then update the compiler stats option using this flag?

I think we don't need this option, default value is stats.colors (https://github.com/webpack/webpack/blob/master/lib/stats/DefaultStatsPresetPlugin.js#L168), webpack-cli should have --stats-colors (or --colors) flag to enable or disable it, it is out of scope webpack-dev-server

alexander-akait · 2020-06-30T11:32:56Z

bin/options.js

-    default: function supportsColor() {
-      // Use `require('supports-color').stdout` for supports-color >= 5.0.0.
-      // See https://github.com/webpack/webpack-dev-server/pull/1555.
-      return require('supports-color').stdout;


Do not forget to remove supports-color

Done here: https://github.com/webpack/webpack-dev-server/pull/2660/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2L62

alexander-akait · 2020-06-30T15:45:48Z

lib/utils/createConfig.js

    // eslint-disable-next-line
-    options.devMiddleware.publicPath =
+    options.dev.publicPath =
      (firstWpOpt.output && firstWpOpt.output.publicPath) || '';


I think it is really bad idea smile But let's resolve it on the other PR

alexander-akait · 2020-07-01T10:40:19Z

@Loonride unstable CI?

knagaitsev · 2020-07-01T18:49:19Z

After this PR we need to:

Change how publicPath is handled
Look deeper into test instability (puppeteer)
Create checks to ensure ports are not reused from the testing ports map
Investigate why webpack compilation calls the done hook many times if the compiler starts directly after an fs.writeFileSync of a CSS file in the compilation (it works fine if there is a 1 second delay between file writing and compilation start)

knagaitsev · 2020-07-01T18:51:03Z

test/e2e/Socket-injection.test.js

@@ -8,7 +8,7 @@ const express = require('express');
 const request = require('supertest');
 const testServer = require('../helpers/test-server');
 const runBrowser = require('../helpers/run-browser');
-const port = require('../ports-map').WebsocketClient;
+const port = require('../ports-map').SocketInjection;


This port was already being used which was causing some instability

knagaitsev · 2020-07-01T18:51:24Z

test/server/contentBasePublicPath-option.test.js

@@ -4,7 +4,7 @@ const path = require('path');
 const request = require('supertest');
 const testServer = require('../helpers/test-server');
 const config = require('../fixtures/contentbase-config/webpack.config');
-const port = require('../ports-map')['contentBase-option'];
+const port = require('../ports-map')['contentBasePublicPath-option'];


Same with this port

alexander-akait · 2020-07-02T11:41:47Z

@Loonride Great! But still failed on macOS node-10-canary 😞 Very strange

knagaitsev · 2020-07-02T15:36:08Z

@Loonride Great! But still failed on macOS node-10-canary Very strange

@evilebottnawi The remaining instability seems to be from puppeteer. I will look closely at that later. Does everything else look good in this PR?

chore(server): upgrade dev middleware and change options

f192a40

knagaitsev requested review from alexander-akait and hiroppy as code owners June 25, 2020 07:08

alexander-akait reviewed Jun 25, 2020

View reviewed changes

test(server): change noop invalidate test

cab2924

knagaitsev commented Jun 26, 2020

View reviewed changes

hiroppy approved these changes Jun 26, 2020

View reviewed changes

knagaitsev added 2 commits June 27, 2020 15:30

chore(middleware): change how mime types work and use of middleware api

78ebd08

chore(middleware): fix routes functionality with middleware

390da6c

knagaitsev commented Jun 28, 2020

View reviewed changes

knagaitsev force-pushed the chore/upgrade-dev-middleware branch from 969b1fc to 390da6c Compare June 28, 2020 16:55

knagaitsev added 2 commits June 28, 2020 11:58

test(options): update create config test for public path

8aca25b

test(options): update headers test

7489d41

knagaitsev added 9 commits June 29, 2020 16:13

refactor(server): remove stats option and use compiler stats option

1d2dad9

test(options): update options test and remove one error message

bef4800

test(server): update normalizeOptions test

717bec4

test(server): update createConfig test

b462d2d

test(server): update validation test and change tested option

4d26b5d

test(server): update multi compiler test

6c927c7

test(cli): remove color cli test

22d97d5

chore(deps): remove supports color

99e31d9

test(server): update universal compiler test

3c0f598

knagaitsev commented Jun 29, 2020

View reviewed changes

alexander-akait reviewed Jun 30, 2020

View reviewed changes

knagaitsev added 4 commits June 30, 2020 10:32

chore(server): add headers option back and update tests

948a4bc

test(server): change title of header test

3f0f0b8

test(server): update tests for headers option

86f6b2a

chore(server): change devMiddleware name to dev

121d999

alexander-akait reviewed Jun 30, 2020

View reviewed changes

knagaitsev added 6 commits June 30, 2020 10:49

chore(deps): upgrade webpack dev middleware for patch release

db4903e

test(e2e): add delay between writing of css files and compilation

c8aec00

test(server): fix stats tests for webpack 5

b657eb1

test(server): fix stats option test for webpack5

157a26a

test(server): fix stats test value

80a34c1

fix(server): ignore stats objects that are empty

3319b0b

knagaitsev force-pushed the chore/upgrade-dev-middleware branch from 686af31 to 3319b0b Compare July 1, 2020 18:39

test(server): fix test stability by adding unique port mappings

30b3443

knagaitsev commented Jul 1, 2020

View reviewed changes

alexander-akait approved these changes Jul 2, 2020

View reviewed changes

alexander-akait merged commit 8f6c8f4 into webpack:v4 Jul 2, 2020

ylemkimon mentioned this pull request Nov 20, 2020

fix(utils): remove socket status and log options.dev.publicPath only if specified #2849

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade dev middleware #2660

chore: upgrade dev middleware #2660

knagaitsev commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

alexander-akait Jun 25, 2020

knagaitsev Jun 25, 2020

hiroppy Jun 26, 2020

alexander-akait Jun 26, 2020

alexander-akait Jun 26, 2020

knagaitsev Jun 26, 2020

hiroppy Jun 26, 2020

alexander-akait commented Jun 26, 2020

knagaitsev commented Jun 26, 2020 •

edited

Loading

knagaitsev Jun 28, 2020 •

edited

Loading

alexander-akait Jun 29, 2020

alexander-akait commented Jun 29, 2020

alexander-akait commented Jun 29, 2020 •

edited

Loading

knagaitsev commented Jun 29, 2020

knagaitsev Jun 29, 2020

alexander-akait Jun 30, 2020 •

edited

Loading

alexander-akait Jun 30, 2020

knagaitsev Jun 30, 2020

alexander-akait Jun 30, 2020

alexander-akait commented Jul 1, 2020

knagaitsev commented Jul 1, 2020 •

edited

Loading

knagaitsev Jul 1, 2020

knagaitsev Jul 1, 2020

alexander-akait commented Jul 2, 2020

knagaitsev commented Jul 2, 2020

chore: upgrade dev middleware #2660

chore: upgrade dev middleware #2660

Conversation

knagaitsev commented Jun 25, 2020 • edited Loading

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Breaking Changes

Additional Info

codecov bot commented Jun 25, 2020 • edited Loading

Codecov Report

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

alexander-akait commented Jun 26, 2020

knagaitsev commented Jun 26, 2020 • edited Loading

knagaitsev Jun 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jun 29, 2020

alexander-akait commented Jun 29, 2020 • edited Loading

knagaitsev commented Jun 29, 2020

Choose a reason for hiding this comment

alexander-akait Jun 30, 2020 • edited Loading

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

alexander-akait commented Jul 1, 2020

knagaitsev commented Jul 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jul 2, 2020

knagaitsev commented Jul 2, 2020

knagaitsev commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

knagaitsev commented Jun 26, 2020 •

edited

Loading

knagaitsev Jun 28, 2020 •

edited

Loading

alexander-akait commented Jun 29, 2020 •

edited

Loading

alexander-akait Jun 30, 2020 •

edited

Loading

knagaitsev commented Jul 1, 2020 •

edited

Loading