Skip to content

chore: upgrade dev middleware #2660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f192a40
chore(server): upgrade dev middleware and change options
knagaitsev Jun 25, 2020
cab2924
test(server): change noop invalidate test
knagaitsev Jun 26, 2020
78ebd08
chore(middleware): change how mime types work and use of middleware api
knagaitsev Jun 27, 2020
390da6c
chore(middleware): fix routes functionality with middleware
knagaitsev Jun 27, 2020
8aca25b
test(options): update create config test for public path
knagaitsev Jun 28, 2020
7489d41
test(options): update headers test
knagaitsev Jun 28, 2020
1d2dad9
refactor(server): remove stats option and use compiler stats option
knagaitsev Jun 29, 2020
bef4800
test(options): update options test and remove one error message
knagaitsev Jun 29, 2020
717bec4
test(server): update normalizeOptions test
knagaitsev Jun 29, 2020
b462d2d
test(server): update createConfig test
knagaitsev Jun 29, 2020
4d26b5d
test(server): update validation test and change tested option
knagaitsev Jun 29, 2020
6c927c7
test(server): update multi compiler test
knagaitsev Jun 29, 2020
22d97d5
test(cli): remove color cli test
knagaitsev Jun 29, 2020
99e31d9
chore(deps): remove supports color
knagaitsev Jun 29, 2020
3c0f598
test(server): update universal compiler test
knagaitsev Jun 29, 2020
948a4bc
chore(server): add headers option back and update tests
knagaitsev Jun 30, 2020
3f0f0b8
test(server): change title of header test
knagaitsev Jun 30, 2020
86f6b2a
test(server): update tests for headers option
knagaitsev Jun 30, 2020
121d999
chore(server): change devMiddleware name to dev
knagaitsev Jun 30, 2020
db4903e
chore(deps): upgrade webpack dev middleware for patch release
knagaitsev Jun 30, 2020
c8aec00
test(e2e): add delay between writing of css files and compilation
knagaitsev Jun 30, 2020
b657eb1
test(server): fix stats tests for webpack 5
knagaitsev Jun 30, 2020
157a26a
test(server): fix stats option test for webpack5
knagaitsev Jun 30, 2020
80a34c1
test(server): fix stats test value
knagaitsev Jun 30, 2020
3319b0b
fix(server): ignore stats objects that are empty
knagaitsev Jun 30, 2020
30b3443
test(server): fix test stability by adding unique port mappings
knagaitsev Jul 1, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions bin/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,6 @@ const options = {
describe: 'Open default browser with the specified page',
requiresArg: true,
},
color: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@alexander-akait alexander-akait Jun 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

type: 'boolean',
alias: 'colors',
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not forget to remove supports-color

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

},
group: DISPLAY_GROUP,
describe: 'Enables/Disables colors on the console',
},
'client-logging': {
type: 'string',
group: DISPLAY_GROUP,
Expand Down
8 changes: 6 additions & 2 deletions bin/webpack-dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const setupExitSignals = require('../lib/utils/setupExitSignals');
const colors = require('../lib/utils/colors');
const processOptions = require('../lib/utils/processOptions');
const getVersions = require('../lib/utils/getVersions');
const getColorsOption = require('../lib/utils/getColorsOption');
const options = require('./options');

let server;
Expand Down Expand Up @@ -87,11 +88,14 @@ const config = require(convertArgvPath)(yargs, argv, {
function startDevServer(config, options) {
let compiler;

const configArr = config instanceof Array ? config : [config];
const statsColors = getColorsOption(configArr);

try {
compiler = webpack(config);
} catch (err) {
if (err instanceof webpack.WebpackOptionsValidationError) {
console.error(colors.error(options.stats.colors, err.message));
console.error(colors.error(statsColors, err.message));
// eslint-disable-next-line no-process-exit
process.exit(1);
}
Expand All @@ -104,7 +108,7 @@ function startDevServer(config, options) {
serverData.server = server;
} catch (err) {
if (err.name === 'ValidationError') {
console.error(colors.error(options.stats.colors, err.message));
console.error(colors.error(statsColors, err.message));
// eslint-disable-next-line no-process-exit
process.exit(1);
}
Expand Down
35 changes: 21 additions & 14 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const compress = require('compression');
const serveIndex = require('serve-index');
const webpack = require('webpack');
const webpackDevMiddleware = require('webpack-dev-middleware');
const getFilenameFromUrl = require('webpack-dev-middleware/dist/utils/getFilenameFromUrl')
.default;
const validateOptions = require('schema-utils');
const isAbsoluteUrl = require('is-absolute-url');
const normalizeOptions = require('./utils/normalizeOptions');
Expand All @@ -30,6 +32,9 @@ const createDomain = require('./utils/createDomain');
const runBonjour = require('./utils/runBonjour');
const routes = require('./utils/routes');
const getSocketServerImplementation = require('./utils/getSocketServerImplementation');
const getCompilerConfigArray = require('./utils/getCompilerConfigArray');
const getStatsOption = require('./utils/getStatsOption');
const getColorsOption = require('./utils/getColorsOption');
const schema = require('./options.json');

if (!process.env.WEBPACK_DEV_SERVER) {
Expand Down Expand Up @@ -146,11 +151,9 @@ class Server {

setupDevMiddleware() {
// middleware for serving webpack bundle
this.middleware = webpackDevMiddleware(
this.middleware = webpackDevMiddleware.default(
this.compiler,
Object.assign({}, this.options, {
logger: this.logger,
})
this.options.dev
);
}

Expand Down Expand Up @@ -666,12 +669,10 @@ class Server {
const suffix = '/';
const uri = `${createDomain(this.options, this.listeningApp)}${suffix}`;

status(
uri,
this.options,
this.logger,
this.options.stats && this.options.stats.colors
);
const configArr = getCompilerConfigArray(this.compiler);
const colors = getColorsOption(configArr);

status(uri, this.options, this.logger, colors);
}

listen(port, hostname, fn) {
Expand Down Expand Up @@ -732,8 +733,10 @@ class Server {
getStats(statsObj) {
const stats = Server.DEFAULT_STATS;

if (this.options.stats.warningsFilter) {
stats.warningsFilter = this.options.stats.warningsFilter;
const configArr = getCompilerConfigArray(this.compiler);
const statsOption = getStatsOption(configArr);
if (typeof statsOption === 'object' && statsOption.warningsFilter) {
stats.warningsFilter = statsOption.warningsFilter;
}

return statsObj.toJson(stats);
Expand Down Expand Up @@ -861,8 +864,12 @@ class Server {
const _path = req.path;

try {
const isFile = this.middleware.fileSystem
.statSync(this.middleware.getFilenameFromUrl(`${_path}.js`))
const filename = getFilenameFromUrl(
this.middleware.context,
`${_path}.js`
);
const isFile = this.middleware.context.outputFileSystem
.statSync(filename)
.isFile();

if (!isFile) {
Expand Down
62 changes: 7 additions & 55 deletions lib/options.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@
}
]
},
"dev": {
"type": "object"
},
"disableHostCheck": {
"type": "boolean"
},
"fs": {
"type": "object"
},
"headers": {
"type": "object"
},
Expand Down Expand Up @@ -176,9 +176,6 @@
}
]
},
"index": {
"type": "string"
},
"injectClient": {
"anyOf": [
{
Expand All @@ -202,9 +199,6 @@
"liveReload": {
"type": "boolean"
},
"mimeTypes": {
"type": "object"
},
"onAfterSetupMiddleware": {
"instanceof": "Function"
},
Expand Down Expand Up @@ -302,44 +296,18 @@
"public": {
"type": "string"
},
"publicPath": {
"type": "string"
},
"requestCert": {
"type": "boolean"
},
"serveIndex": {
"type": "boolean"
},
"serverSideRender": {
"type": "boolean"
},
"socket": {
"type": "string"
},
"staticOptions": {
"type": "object"
},
"stats": {
"anyOf": [
{
"type": "object"
},
{
"type": "boolean"
},
{
"enum": [
"none",
"errors-only",
"errors-warnings",
"minimal",
"normal",
"verbose"
]
}
]
},
"transportMode": {
"anyOf": [
{
Expand Down Expand Up @@ -374,16 +342,6 @@
},
"watchOptions": {
"type": "object"
},
"writeToDisk": {
"anyOf": [
{
"type": "boolean"
},
{
"instanceof": "Function"
}
]
}
},
"errorMessage": {
Expand All @@ -393,19 +351,17 @@
"client": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverclient)",
"compress": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devservercompress)",
"contentBase": "should be {Number|String|Array} (https://webpack.js.org/configuration/dev-server/#devservercontentbase)",
"dev": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverdev-)",
"disableHostCheck": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverdisablehostcheck)",
"fs": "should be {Object} (https://github.com/webpack/webpack-dev-middleware#fs)",
"headers": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverheaders-)",
"headers": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverheaders)",
"historyApiFallback": "should be {Boolean|Object} (https://webpack.js.org/configuration/dev-server/#devserverhistoryapifallback)",
"host": "should be {String|Null} (https://webpack.js.org/configuration/dev-server/#devserverhost)",
"hot": "should be {Boolean|String} (https://webpack.js.org/configuration/dev-server/#devserverhot)",
"http2": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverhttp2)",
"https": "should be {Object|Boolean} (https://webpack.js.org/configuration/dev-server/#devserverhttps)",
"index": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserverindex)",
"injectClient": "should be {Boolean|Function} (https://webpack.js.org/configuration/dev-server/#devserverinjectclient)",
"injectHot": "should be {Boolean|Function} (https://webpack.js.org/configuration/dev-server/#devserverinjecthot)",
"liveReload": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverlivereload-)",
"mimeTypes": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devservermimetypes-)",
"liveReload": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverlivereload)",
"onAfterSetupMiddleware": "should be {Function} (https://webpack.js.org/configuration/dev-server/#devserverafter)",
"onBeforeSetupMiddleware": "should be {Function} (https://webpack.js.org/configuration/dev-server/#devserverbefore)",
"onListening": "should be {Function} (https://webpack.js.org/configuration/dev-server/#onlistening)",
Expand All @@ -417,19 +373,15 @@
"progress": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverprogress---cli-only)",
"proxy": "should be {Object|Array} (https://webpack.js.org/configuration/dev-server/#devserverproxy)",
"public": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserverpublic)",
"publicPath": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserverpublicpath-)",
"requestCert": "should be {Boolean}",
"contentBasePublicPath": "should be {String|Array} (https://webpack.js.org/configuration/dev-server/#devservercontentbasepublicpath)",
"serveIndex": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverserveindex)",
"serverSideRender": "should be {Boolean} (https://github.com/webpack/webpack-dev-middleware#serversiderender)",
"socket": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserversocket)",
"staticOptions": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverstaticoptions)",
"stats": "should be {Object|Boolean} (https://webpack.js.org/configuration/dev-server/#devserverstats-)",
"transportMode": "should be {String|Object} (https://webpack.js.org/configuration/dev-server/#devservertransportmode)",
"useLocalIp": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserveruselocalip)",
"watchContentBase": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverwatchcontentbase)",
"watchOptions": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverwatchoptions-)",
"writeToDisk": "should be {Boolean|Function} (https://webpack.js.org/configuration/dev-server/#devserverwritetodisk-)"
"watchOptions": "should be {Object} (https://webpack.js.org/configuration/dev-server/#devserverwatchoptions)"
}
},
"additionalProperties": false
Expand Down
27 changes: 7 additions & 20 deletions lib/utils/createConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,18 @@ function createConfig(config, argv, { port }) {
options.overlay = argv.overlay;
}

if (!options.publicPath) {
options.dev = options.dev || {};

if (!options.dev.publicPath) {
// eslint-disable-next-line
options.publicPath =
options.dev.publicPath =
(firstWpOpt.output && firstWpOpt.output.publicPath) || '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


if (
!isAbsoluteUrl(String(options.publicPath)) &&
options.publicPath[0] !== '/'
!isAbsoluteUrl(String(options.dev.publicPath)) &&
options.dev.publicPath[0] !== '/'
) {
options.publicPath = `/${options.publicPath}`;
options.dev.publicPath = `/${options.dev.publicPath}`;
}
}

Expand Down Expand Up @@ -111,21 +113,6 @@ function createConfig(config, argv, { port }) {
options.watchContentBase = true;
}

if (!options.stats) {
options.stats = defaultTo(firstWpOpt.stats, {
cached: false,
cachedAssets: false,
});
}

if (
typeof options.stats === 'object' &&
typeof options.stats.colors === 'undefined' &&
argv.color
) {
options.stats = Object.assign({}, options.stats, { colors: argv.color });
}

if (argv.https) {
options.https = true;
}
Expand Down
15 changes: 15 additions & 0 deletions lib/utils/getColorsOption.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

const getStatsOption = require('./getStatsOption');

function getColorsOption(configArr) {
const statsOption = getStatsOption(configArr);
let colors = false;
if (typeof statsOption === 'object' && statsOption.colors) {
colors = statsOption.colors;
}

return colors;
}

module.exports = getColorsOption;
8 changes: 8 additions & 0 deletions lib/utils/getCompilerConfigArray.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

function getCompilerConfigArray(compiler) {
const compilers = compiler.compilers ? compiler.compilers : [compiler];
return compilers.map((comp) => comp.options);
}

module.exports = getCompilerConfigArray;
16 changes: 16 additions & 0 deletions lib/utils/getStatsOption.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';

function getStatsOption(configArr) {
const isEmptyObject = (val) =>
typeof val === 'object' && Object.keys(val).length === 0;

// in webpack@4 stats will not be defined if not provided,
// but in webpack@5 it will be an empty object
const statsConfig = configArr.find(
(conf) =>
typeof conf === 'object' && conf.stats && !isEmptyObject(conf.stats)
);
return statsConfig ? statsConfig.stats : {};
}

module.exports = getStatsOption;
6 changes: 2 additions & 4 deletions lib/utils/normalizeOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ function normalizeOptions(compiler, options) {
: true;
options.liveReload =
typeof options.liveReload !== 'undefined' ? options.liveReload : true;
options.stats =
options.stats && Object.keys(options.stats).length !== 0
? options.stats
: {};

// normalize transportMode option
if (typeof options.transportMode === 'undefined') {
Expand Down Expand Up @@ -49,6 +45,8 @@ function normalizeOptions(compiler, options) {
options.client.path = `/${
options.client.path ? options.client.path.replace(/^\/|\/$/g, '') : 'ws'
}`;

options.dev = options.dev || {};
}

module.exports = normalizeOptions;
Loading