Skip to content
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

Additional follow-up fix for #1704 to remove remaining warnings #1736

Merged
5 changes: 5 additions & 0 deletions packages/razzle-dev-utils/devServerMajor.js
@@ -0,0 +1,5 @@
'use strict';

const devserverPkg = require('webpack-dev-server/package.json');

module.exports = devserverPkg.version ? parseInt(devserverPkg.version[0]) : 3;
10 changes: 9 additions & 1 deletion packages/razzle-dev-utils/webpackHotDevClient.js
Expand Up @@ -12,11 +12,19 @@
var SockJS = require('sockjs-client');
var stripAnsi = require('strip-ansi');
var url = require('url');
var createSocketUrl = require('webpack-dev-server/client/utils/createSocketUrl');
var launchEditorEndpoint = require('react-dev-utils/launchEditorEndpoint');
var devServerMajorVersion = require('./devServerMajor');
var formatWebpackMessages = require('./formatWebpackMessages');
var ErrorOverlay = require('react-error-overlay');

var createSocketUrl;
if (devServerMajorVersion > 3) {
// The path changed with v4
createSocketUrl = require('webpack-dev-server/client/utils/createSocketURL');
} else {
createSocketUrl = require('webpack-dev-server/client/utils/createSocketUrl');
}

var socketUrl = createSocketUrl();
var parsedSocketUrl = url.parse(socketUrl);

Expand Down
17 changes: 8 additions & 9 deletions packages/razzle/config/createConfigAsync.js
Expand Up @@ -24,10 +24,7 @@ const logger = require('razzle-dev-utils/logger');
const razzlePaths = require('./paths');
const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');
const webpackMajor = require('razzle-dev-utils/webpackMajor');
const devserverPkg = require('webpack-dev-server/package.json');

// Parse the first character from the `x.y.z` version notation (i.e. major version)
const devServerMajorVersion = parseInt(devserverPkg.version[0]);
const devServerMajorVersion = require('razzle-dev-utils/devServerMajor');

const hasPostCssConfigTest = () => {
try {
Expand Down Expand Up @@ -850,6 +847,7 @@ module.exports = (
// See https://github.com/facebookincubator/create-react-app/issues/387.
disableDotRule: true,
},
hot: true,

Choose a reason for hiding this comment

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

👍

host: dotenv.raw.HOST,
port: devServerPort,
};
Expand Down Expand Up @@ -880,7 +878,6 @@ module.exports = (
disableHostCheck: true,
clientLogLevel: 'none', // Enable gzip compression of generated files.
publicPath: clientPublicPath,
hot: true,
noInfo: true,
overlay: false,
quiet: true, // By default files from `contentBase` will not trigger a page reload.
Expand All @@ -897,10 +894,12 @@ module.exports = (
// Add client-only development plugins
config.plugins = [
...config.plugins,
new webpack.HotModuleReplacementPlugin({
// set this true will break HtmlWebpackPlugin
multiStep: !clientOnly,
}),
devServerMajorVersion > 3
? null // avoid warning since v4 automatically adds the HRM plugin when `hot` is true
: new webpack.HotModuleReplacementPlugin({
// set this true will break HtmlWebpackPlugin
multiStep: !clientOnly,
}),
shouldUseReactRefresh
? new ReactRefreshWebpackPlugin({
overlay: {
Expand Down
27 changes: 19 additions & 8 deletions packages/razzle/scripts/start.js
Expand Up @@ -14,6 +14,7 @@ const logger = require('razzle-dev-utils/logger');
const setPorts = require('razzle-dev-utils/setPorts');
const chalk = require('chalk');
const terminate = require('terminate');
const devServerMajorVersion = require('razzle-dev-utils/devServerMajor');

let verbose = false;

Expand Down Expand Up @@ -172,20 +173,30 @@ function main() {
// This will actually run on a different port than the users app.
clientDevServer = new devServer(
clientCompiler,
Object.assign(clientConfig.devServer, { verbose: verbose })
Object.assign(clientConfig.devServer, { verbose, port }),
);
// Start Webpack-dev-server
clientDevServer.listen(port, err => {
if (err) {
logger.error(err);
}
});
if (devServerMajorVersion > 3) {
// listen was deprecated in v4 and causes issues when used, switch to its replacement
clientDevServer.start();

Choose a reason for hiding this comment

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

You could use startCallback if you want to have feature parity on the error handler.

const startCallback = (error) => err && logger.error(err);
clientDevServer.startCallback(startCallback , startCallback);

Choose a reason for hiding this comment

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

You can add the same (not parity) to stop with stopCallback, which I believe will only trigger if a middleware is unable to close.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the source code for startCallback and stopCallback currently, (err) will always be null... But I can add it in case things change in the future. Or maybe I'll just add .catch() handlers to both start and stop instead... since ultimately we just want to deal with error that are thrown

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok... Pushed code to support this, assuming that the new issue I filed on webpack-dev-server gets addressed.

} else {
// Start Webpack-dev-server
clientDevServer.listen(port, err => {
if (err) {
logger.error(err);
}
});
}
}

['SIGINT', 'SIGTERM'].forEach(sig => {
process.on(sig, () => {
if (clientDevServer) {
clientDevServer.close();
if (devServerMajorVersion > 3) {
// close was deprecated in v4, switch to its replacement
clientDevServer.stop();
} else {
clientDevServer.close();
}
}
if (watching) {
watching.close();
Expand Down