Skip to content

feat(client): add clientMode option #1977

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 8 commits into from
Jun 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 11 additions & 2 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ class Server {

validateOptions(schema, options, 'webpack Dev Server');

updateCompiler(compiler, options);

this.compiler = compiler;
this.options = options;

Expand All @@ -67,13 +65,24 @@ class Server {

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

// set serverMode default
if (this.options.serverMode === undefined) {
this.options.serverMode = 'sockjs';
} else {
this.log.warn(
'serverMode is an experimental option, meaning its usage could potentially change without warning'
);
}
// set clientMode default
if (this.options.clientMode === undefined) {
this.options.clientMode = 'sockjs';
} else {
this.log.warn(
'clientMode is an experimental option, meaning its usage could potentially change without warning'
);
}

updateCompiler(this.compiler, this.options);
Copy link
Member

Choose a reason for hiding this comment

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

Why we move this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@evilebottnawi I reorganized it to have the default setting for serverMode and clientMode together above that. updateCompiler uses this.options.clientMode, so the default setting for it must be set before updateCompiler is called. It can be switched back, I just thought it would be nice to have default setting for serverMode and clientMode right next to one another


// this.SocketServerImplementation is a class, so it must be instantiated before use
this.socketServerImplementation = getSocketServerImplementation(
Expand Down
6 changes: 5 additions & 1 deletion lib/options.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@
"warning"
]
},
"clientMode": {
"type": "string"
},
"compress": {
"type": "boolean"
},
Expand Down Expand Up @@ -390,6 +393,7 @@
"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 @@ -430,7 +434,7 @@
"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-)",
"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 Down
36 changes: 36 additions & 0 deletions lib/utils/getSocketClientPath.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';

function getSocketClientPath(options) {
let ClientImplementation;
let clientImplFound = true;
switch (typeof options.clientMode) {
case 'string':
// could be 'sockjs', in the future 'ws', or a path that should be required
if (options.clientMode === 'sockjs') {
// eslint-disable-next-line global-require
ClientImplementation = require('../../client/clients/SockJSClient');
} else {
try {
// eslint-disable-next-line global-require, import/no-dynamic-require
ClientImplementation = require(options.clientMode);
} catch (e) {
clientImplFound = false;
}
}
break;
default:
clientImplFound = false;
}

if (!clientImplFound) {
throw new Error(
"clientMode must be a string denoting a default implementation (e.g. 'sockjs') or a full path to " +
'a JS file which exports a class extending BaseClient (webpack-dev-server/client-src/clients/BaseClient) ' +
'via require.resolve(...)'
);
}

return ClientImplementation.getClientPath(options);
}

module.exports = getSocketClientPath;
6 changes: 2 additions & 4 deletions lib/utils/updateCompiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
const webpack = require('webpack');
const addEntries = require('./addEntries');
const getSocketClientPath = require('./getSocketClientPath');

function updateCompiler(compiler, options) {
if (options.inline !== false) {
Expand Down Expand Up @@ -50,10 +51,7 @@ function updateCompiler(compiler, options) {
compiler.hooks.entryOption.call(config.context, config.entry);

const providePlugin = new webpack.ProvidePlugin({
// SockJSClient.getClientPath(options)
__webpack_dev_server_client__: require.resolve(
'../../client/clients/SockJSClient.js'
),
__webpack_dev_server_client__: getSocketClientPath(options),
});
providePlugin.apply(compiler);
});
Expand Down
82 changes: 82 additions & 0 deletions test/e2e/ClientMode.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict';

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;

describe('clientMode', () => {
describe('sockjs', () => {
beforeAll((done) => {
const options = {
port,
host: '0.0.0.0',
inline: true,
clientMode: 'sockjs',
};
testServer.startAwaitingCompilation(config, options, done);
});

describe('on browser client', () => {
it('logs as usual', (done) => {
runBrowser().then(({ page, browser }) => {
const res = [];
page.goto(`http://localhost:${port}/main`);
page.on('console', ({ _text }) => {
res.push(_text);
});

setTimeout(() => {
testServer.close(() => {
// make sure the client gets the close message
setTimeout(() => {
browser.close().then(() => {
expect(res).toMatchSnapshot();
done();
});
}, 1000);
});
}, 3000);
});
});
});
});

describe('custom client', () => {
beforeAll((done) => {
const options = {
port,
host: '0.0.0.0',
inline: true,
clientMode: require.resolve(
'../fixtures/custom-client/CustomSockJSClient'
),
};
testServer.startAwaitingCompilation(config, options, done);
});

describe('on browser client', () => {
it('logs additional messages to console', (done) => {
runBrowser().then(({ page, browser }) => {
const res = [];
page.goto(`http://localhost:${port}/main`);
page.on('console', ({ _text }) => {
res.push(_text);
});

setTimeout(() => {
testServer.close(() => {
// make sure the client gets the close message
setTimeout(() => {
browser.close().then(() => {
expect(res).toMatchSnapshot();
done();
});
}, 1000);
});
}, 3000);
});
});
});
});
});
22 changes: 22 additions & 0 deletions test/e2e/__snapshots__/ClientMode.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`clientMode custom client on browser client logs additional messages to console 1`] = `
Array [
"Hey.",
"open",
"liveReload",
"[WDS] Live Reloading enabled.",
"hash",
"ok",
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't onClose called?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hiroppy testServer.close happens after the snapshot is tested, so the client is not closed before that. I could maybe rearrange that to test onClose as well.

"close",
"[WDS] Disconnected!",
]
`;

exports[`clientMode sockjs on browser client logs as usual 1`] = `
Array [
"Hey.",
"[WDS] Live Reloading enabled.",
"[WDS] Disconnected!",
]
`;
41 changes: 41 additions & 0 deletions test/fixtures/custom-client/CustomSockJSClient.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict';

/* eslint-disable
no-unused-vars
*/
const SockJS = require('sockjs-client/dist/sockjs');
const BaseClient = require('../../../client/clients/BaseClient');

module.exports = class SockJSClient extends BaseClient {
constructor(url) {
super();
this.sock = new SockJS(url);
}

static getClientPath(options) {
return require.resolve('./CustomSockJSClient');
}

onOpen(f) {
this.sock.onopen = () => {
console.log('open');
f();
};
}

onClose(f) {
this.sock.onclose = () => {
console.log('close');
f();
};
}

// call f with the message string as the first argument
onMessage(f) {
this.sock.onmessage = (e) => {
const obj = JSON.parse(e.data);
console.log(obj.type);
f(e.data);
};
}
};
4 changes: 4 additions & 0 deletions test/options.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ describe('options', () => {
],
failure: ['whoops!'],
},
clientMode: {
success: ['sockjs', require.resolve('../client/clients/SockJSClient')],
failure: [false],
},
compress: {
success: [true],
failure: [''],
Expand Down
2 changes: 2 additions & 0 deletions test/ports-map.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

// test-file-name: the number of ports
// important: new port mappings must be added to the bottom of this list
const portsList = {
cli: 2,
sockJSClient: 1,
Expand Down Expand Up @@ -37,6 +38,7 @@ const portsList = {
ProvidePlugin: 1,
WebsocketClient: 1,
WebsocketServer: 1,
ClientMode: 1,
};

let startPort = 8079;
Expand Down
50 changes: 50 additions & 0 deletions test/server/utils/getSocketClientPath.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict';

const getSocketClientPath = require('../../../lib/utils/getSocketClientPath');
// 'npm run prepare' must be done for this test to pass
const sockjsClientPath = require.resolve(
'../../../client/clients/SockJSClient'
);
const baseClientPath = require.resolve('../../../client/clients/BaseClient');

describe('getSocketClientPath', () => {
it("should work with clientMode: 'sockjs'", () => {
let result;

expect(() => {
result = getSocketClientPath({
clientMode: 'sockjs',
});
}).not.toThrow();

expect(result).toEqual(sockjsClientPath);
});

it('should work with clientMode: SockJSClient full path', () => {
let result;

expect(() => {
result = getSocketClientPath({
clientMode: sockjsClientPath,
});
}).not.toThrow();

expect(result).toEqual(sockjsClientPath);
});

it('should throw with clientMode: bad path', () => {
expect(() => {
getSocketClientPath({
clientMode: '/bad/path/to/implementation',
});
}).toThrow(/clientMode must be a string/);
});

it('should throw with clientMode: unimplemented client', () => {
expect(() => {
getSocketClientPath({
clientMode: baseClientPath,
});
}).toThrow('Client needs implementation');
});
});