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.

Sign up for GitHub

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

Jump to bottom

feat(server): add transportMode #2116

Merged
merged 8 commits into from Aug 8, 2019
Merged

feat(server): add transportMode #2116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ae7d162
feat(server): add transport mode
knagaitsev Jul 11, 2019
d9afb64
fix(transport): update transport e2e test and rename tests
knagaitsev Jul 11, 2019
1b3e78d
test(options): fixed options test
knagaitsev Jul 11, 2019
c648678
refactor(server): move transport mode default setting into helper
knagaitsev Jul 23, 2019
a0d8a9e
Merge branch 'master' into transportMode-option
evilebottnawi Jul 30, 2019
90790a4
Merge branch 'master' into transportMode-option
evilebottnawi Jul 30, 2019
be8e864
Merge branch 'master' into transportMode-option
evilebottnawi Aug 6, 2019
aa17f7c
Merge branch 'master' into transportMode-option
evilebottnawi Aug 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 3 additions & 9 deletions lib/Server.js
View file
Open in desktop
Expand Up @@ -61,15 +61,9 @@ class Server {

this.log = _log || createLogger(options);

if (this.options.serverMode !== undefined) {
if (this.options.transportMode !== undefined) {
this.log.warn(
'serverMode is an experimental option, meaning its usage could potentially change without warning'
);
}

if (this.options.clientMode !== undefined) {
this.log.warn(
'clientMode is an experimental option, meaning its usage could potentially change without warning'
'transportMode is an experimental option, meaning its usage could potentially change without warning'
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can move this in util, like getTransportMode(options: object): object

);
}

Expand Down Expand Up @@ -672,7 +666,7 @@ class Server {

if (!headers) {
this.log.warn(
'serverMode implementation must pass headers to the callback of onConnection(f) ' +
'transportMode.server implementation must pass headers to the callback of onConnection(f) ' +
'via f(connection, headers) in order for clients to pass a headers security check'
);
}
Expand Down
42 changes: 27 additions & 15 deletions lib/options.json
View file
Open in desktop
Expand Up @@ -48,9 +48,6 @@
"warning"
]
},
"clientMode": {
"type": "string"
},
"compress": {
"type": "boolean"
},
Expand Down Expand Up @@ -303,16 +300,6 @@
"serveIndex": {
"type": "boolean"
},
"serverMode": {
"anyOf": [
{
"type": "string"
},
{
"instanceof": "Function"
}
]
},
"serverSideRender": {
"type": "boolean"
},
Expand Down Expand Up @@ -364,6 +351,32 @@
}
]
},
"transportMode": {
"anyOf": [
{
"type": "object",
"properties": {
"client": {
"type": "string"
},
"server": {
"anyOf": [
{
"type": "string"
},
{
"instanceof": "Function"
}
]
}
},
"additionalProperties": false
},
{
"enum": ["sockjs", "ws"]
}
]
},
"useLocalIp": {
"type": "boolean"
},
Expand Down Expand Up @@ -396,7 +409,6 @@
"ca": "should be {String|Buffer}",
"cert": "should be {String|Buffer}",
"clientLogLevel": "should be {String} and equal to one of the allowed values\n\n [ 'none', 'silent', 'info', 'debug', 'trace', 'error', 'warning', 'warn' ]\n\n (https://webpack.js.org/configuration/dev-server/#devserverclientloglevel)",
"clientMode": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserverclientmode)",
"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)",
"disableHostCheck": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverdisablehostcheck)",
Expand Down Expand Up @@ -438,7 +450,6 @@
"reporter": "should be {Function} (https://github.com/webpack/webpack-dev-middleware#reporter)",
"requestCert": "should be {Boolean}",
"serveIndex": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverserveindex)",
"serverMode": "should be {String|Function} (https://webpack.js.org/configuration/dev-server/#devserverservermode)",
"serverSideRender": "should be {Boolean} (https://github.com/webpack/webpack-dev-middleware#serversiderender)",
"setup": "should be {Function} (https://webpack.js.org/configuration/dev-server/#devserversetup)",
"sockHost": "should be {String|Null} (https://webpack.js.org/configuration/dev-server/#devserversockhost)",
Expand All @@ -447,6 +458,7 @@
"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)",
"warn": "should be {Function}",
"watchContentBase": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserverwatchcontentbase)",
Expand Down
10 changes: 5 additions & 5 deletions lib/utils/getSocketClientPath.js
View file
Open in desktop
Expand Up @@ -3,17 +3,17 @@
function getSocketClientPath(options) {
let ClientImplementation;
let clientImplFound = true;
switch (typeof options.clientMode) {
switch (typeof options.transportMode.client) {
case 'string':
// could be 'sockjs', 'ws', or a path that should be required
if (options.clientMode === 'sockjs') {
if (options.transportMode.client === 'sockjs') {
ClientImplementation = require('../../client/clients/SockJSClient');
} else if (options.clientMode === 'ws') {
} else if (options.transportMode.client === 'ws') {
ClientImplementation = require('../../client/clients/WebsocketClient');
} else {
try {
// eslint-disable-next-line import/no-dynamic-require
ClientImplementation = require(options.clientMode);
ClientImplementation = require(options.transportMode.client);
} catch (e) {
clientImplFound = false;
}
Expand All @@ -25,7 +25,7 @@ function getSocketClientPath(options) {

if (!clientImplFound) {
throw new Error(
"clientMode must be a string denoting a default implementation (e.g. 'sockjs', 'ws') or a full path to " +
"transportMode.client must be a string denoting a default implementation (e.g. 'sockjs', 'ws') or a full path to " +
'a JS file which exports a class extending BaseClient (webpack-dev-server/client-src/clients/BaseClient) ' +
'via require.resolve(...)'
);
Expand Down
12 changes: 6 additions & 6 deletions lib/utils/getSocketServerImplementation.js
View file
Open in desktop
Expand Up @@ -3,17 +3,17 @@
function getSocketServerImplementation(options) {
let ServerImplementation;
let serverImplFound = true;
switch (typeof options.serverMode) {
switch (typeof options.transportMode.server) {
case 'string':
// could be 'sockjs', in the future 'ws', or a path that should be required
if (options.serverMode === 'sockjs') {
if (options.transportMode.server === 'sockjs') {
ServerImplementation = require('../servers/SockJSServer');
} else if (options.serverMode === 'ws') {
} else if (options.transportMode.server === 'ws') {
ServerImplementation = require('../servers/WebsocketServer');
} else {
try {
// eslint-disable-next-line import/no-dynamic-require
ServerImplementation = require(options.serverMode);
ServerImplementation = require(options.transportMode.server);
} catch (e) {
serverImplFound = false;
}
Expand All @@ -22,15 +22,15 @@ function getSocketServerImplementation(options) {
case 'function':
// potentially do more checks here to confirm that the user implemented this properlly
// since errors could be difficult to understand
ServerImplementation = options.serverMode;
ServerImplementation = options.transportMode.server;
break;
default:
serverImplFound = false;
}

if (!serverImplFound) {
throw new Error(
"serverMode must be a string denoting a default implementation (e.g. 'sockjs', 'ws'), a full path to " +
"transportMode.server must be a string denoting a default implementation (e.g. 'sockjs', 'ws'), a full path to " +
'a JS file which exports a class extending BaseServer (webpack-dev-server/lib/servers/BaseServer) ' +
'via require.resolve(...), or the class itself which extends BaseServer'
);
Expand Down
27 changes: 19 additions & 8 deletions lib/utils/normalizeOptions.js
View file
Open in desktop
Expand Up @@ -9,14 +9,25 @@ function normalizeOptions(compiler, options) {
options.contentBase =
options.contentBase !== undefined ? options.contentBase : process.cwd();

// set serverMode default
if (options.serverMode === undefined) {
options.serverMode = 'sockjs';
}

// set clientMode default
if (options.clientMode === undefined) {
options.clientMode = 'sockjs';
// normalize transportMode option
if (options.transportMode === undefined) {
options.transportMode = {
server: 'sockjs',
client: 'sockjs',
};
} else {
switch (typeof options.transportMode) {
case 'string':
options.transportMode = {
server: options.transportMode,
client: options.transportMode,
};
break;
// if not a string, it is an object
default:
options.transportMode.server = options.transportMode.server || 'sockjs';
options.transportMode.client = options.transportMode.client || 'sockjs';
}
}

if (!options.watchOptions) {
Expand Down
7 changes: 3 additions & 4 deletions test/e2e/Client.test.js
View file
Open in desktop
Expand Up @@ -23,18 +23,17 @@ const cssFilePath = resolve(__dirname, '../fixtures/reload-config/main.css');
describe('reload', () => {
const modes = [
{
title: 'hot with default clientMode (sockjs)',
title: 'hot with default transportMode.client (sockjs)',
options: {
hot: true,
},
shouldRefresh: false,
},
{
title: 'hot with clientMode ws',
title: 'hot with transportMode.client ws',
options: {
hot: true,
clientMode: 'ws',
serverMode: require.resolve('../../lib/servers/WebsocketServer'),
transportMode: 'ws',
},
shouldRefresh: false,
},
Expand Down
7 changes: 3 additions & 4 deletions test/e2e/ProvidePlugin.test.js
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ const port = require('../ports-map').ProvidePlugin;
const { beforeBrowserCloseDelay } = require('../helpers/puppeteer-constants');

describe('ProvidePlugin', () => {
describe('inline with default clientMode (sockjs)', () => {
describe('inline with default transportMode.client (sockjs)', () => {
beforeAll((done) => {
const options = {
port,
Expand Down Expand Up @@ -46,14 +46,13 @@ describe('ProvidePlugin', () => {
});
});

describe('inline with clientMode ws', () => {
describe('inline with transportMode.client ws', () => {
beforeAll((done) => {
const options = {
port,
host: '0.0.0.0',
inline: true,
clientMode: 'ws',
serverMode: require.resolve('../../lib/servers/WebsocketServer'),
transportMode: 'ws',
watchOptions: {
poll: true,
},
Expand Down
18 changes: 10 additions & 8 deletions test/e2e/ClientMode.test.js → test/e2e/TransportMode.test.js
View file
Open in desktop
Expand Up @@ -3,33 +3,35 @@
const testServer = require('../helpers/test-server');
const config = require('../fixtures/client-config/webpack.config');
const runBrowser = require('../helpers/run-browser');
const port = require('../ports-map').ClientMode;
const port = require('../ports-map').TransportMode;
const {
initConsoleDelay,
awaitServerCloseDelay,
} = require('../helpers/puppeteer-constants');

describe('clientMode', () => {
describe('transportMode client', () => {
const modes = [
{
title: 'sockjs',
options: {
clientMode: 'sockjs',
transportMode: 'sockjs',
},
},
{
title: 'ws',
options: {
clientMode: 'ws',
serverMode: require.resolve('../../lib/servers/WebsocketServer'),
transportMode: 'ws',
},
},
{
title: 'custom client',
options: {
clientMode: require.resolve(
'../fixtures/custom-client/CustomSockJSClient'
),
transportMode: {
server: 'sockjs',
client: require.resolve(
'../fixtures/custom-client/CustomSockJSClient'
),
},
},
},
];
Expand Down
6 changes: 3 additions & 3 deletions ...e2e/__snapshots__/ClientMode.test.js.snap → .../__snapshots__/TransportMode.test.js.snap
View file
Open in desktop
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`clientMode custom client on browser client logs correctly 1`] = `
exports[`transportMode client custom client on browser client logs correctly 1`] = `
Array [
"Hey.",
"open",
Expand All @@ -13,15 +13,15 @@ Array [
]
`;

exports[`clientMode sockjs on browser client logs correctly 1`] = `
exports[`transportMode client sockjs on browser client logs correctly 1`] = `
Array [
"Hey.",
"[WDS] Live Reloading enabled.",
"[WDS] Disconnected!",
]
`;

exports[`clientMode ws on browser client logs correctly 1`] = `
exports[`transportMode client ws on browser client logs correctly 1`] = `
Array [
"Hey.",
"[WDS] Live Reloading enabled.",
Expand Down