Skip to content

Commit

Permalink
Do not use spdy on Node 10
Browse files Browse the repository at this point in the history
`spdy` is effectively unmaintained, and as a consequence of an
implementation that extensively relies on Node’s non-public APIs, broken
on Node 10 and above. In those cases, only https will be used for now.
Once express supports Node's built-in HTTP/2 support, migrating over to
that should be the best way to go.

Fixes: #1449
Fixes: nodejs/node#21665
  • Loading branch information
addaleax committed Aug 1, 2018
1 parent e1bd264 commit d141150
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion lib/Server.js
Expand Up @@ -5,6 +5,7 @@ require('./polyfills');

const fs = require('fs');
const http = require('http');
const https = require('https');
const path = require('path');
const url = require('url');
const chokidar = require('chokidar');
Expand Down Expand Up @@ -484,7 +485,20 @@ function Server(compiler, options, _log) {
};
}

this.listeningApp = spdy.createServer(options.https, app);
// `spdy` is effectively unmaintained, and as a consequence of an
// implementation that extensively relies on Node’s non-public APIs, broken
// on Node 10 and above. In those cases, only https will be used for now.
// Once express supports Node's built-in HTTP/2 support, migrating over to
// that should be the best way to go.
// The relevant issues are:
// - https://github.com/nodejs/node/issues/21665
// - https://github.com/webpack/webpack-dev-server/issues/1449
// - https://github.com/expressjs/express/issues/3388
if (+process.version.match(/^v(\d+)/[1]) >= 10) {
this.listeningApp = https.createServer(options.https, app);
} else {
this.listeningApp = spdy.createServer(options.https, app);
}
} else {
this.listeningApp = http.createServer(app);
}
Expand Down

0 comments on commit d141150

Please sign in to comment.