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

fix: disable socket injection when hot & liveReload are disabled (V4) #2601

Merged
merged 6 commits into from
May 22, 2020
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
1 change: 1 addition & 0 deletions client-src/default/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ const onSocketMessage = {
},
close() {
log.error('[WDS] Disconnected!');

sendMessage('Close');
},
};
Expand Down
6 changes: 4 additions & 2 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,9 @@ class Server {
this.hostname = hostname;

return this.listeningApp.listen(port, hostname, (err) => {
this.createSocketServer();
if (this.options.hot || this.options.liveReload) {
this.createSocketServer();
}

if (this.options.bonjour) {
runBonjour(this.options);
Expand Down Expand Up @@ -923,7 +925,7 @@ class Server {

const watcher = chokidar.watch(watchPath, watchOptions);
// disabling refreshing on changing the content
if (this.options.liveReload !== false) {
if (this.options.liveReload) {
watcher.on('change', () => {
this.sockWrite(this.sockets, 'content-changed');
});
Expand Down
8 changes: 8 additions & 0 deletions test/client/__snapshots__/index.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`index should run onSocketMessage.close (hot enabled) 1`] = `"[WDS] Disconnected!"`;

exports[`index should run onSocketMessage.close (hot enabled) 2`] = `"Close"`;

exports[`index should run onSocketMessage.close (liveReload enabled) 1`] = `"[WDS] Disconnected!"`;

exports[`index should run onSocketMessage.close (liveReload enabled) 2`] = `"Close"`;

exports[`index should run onSocketMessage.close 1`] = `"[WDS] Disconnected!"`;

exports[`index should run onSocketMessage.close 2`] = `"Close"`;
Expand Down
16 changes: 16 additions & 0 deletions test/client/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,20 @@ describe('index', () => {
expect(log.log.error.mock.calls[0][0]).toMatchSnapshot();
expect(sendMessage.mock.calls[0][0]).toMatchSnapshot();
});

test('should run onSocketMessage.close (hot enabled)', () => {
// enabling hot
onSocketMessage.hot();
onSocketMessage.close();
expect(log.log.error.mock.calls[0][0]).toMatchSnapshot();
expect(sendMessage.mock.calls[0][0]).toMatchSnapshot();
});

test('should run onSocketMessage.close (liveReload enabled)', () => {
// enabling liveReload
onSocketMessage.liveReload();
onSocketMessage.close();
expect(log.log.error.mock.calls[0][0]).toMatchSnapshot();
expect(sendMessage.mock.calls[0][0]).toMatchSnapshot();
});
});
74 changes: 34 additions & 40 deletions test/e2e/ClientOptions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,12 @@ describe('Client console.log', () => {
port: port2,
host: '0.0.0.0',
};
const transportModes = [
{},
{ transportMode: 'sockjs' },
{ transportMode: 'ws' },
];

const cases = [
{
title: 'hot disabled',
Expand All @@ -390,6 +396,13 @@ describe('Client console.log', () => {
liveReload: true,
},
},
{
title: 'liveReload & hot are disabled',
options: {
liveReload: false,
hot: false,
},
},
// TODO: make clientLogLevel work as expected for HMR logs
{
title: 'clientLogLevel is silent',
Expand All @@ -399,48 +412,29 @@ describe('Client console.log', () => {
},
];

cases.forEach(({ title, options }) => {
it(title, (done) => {
const res = [];
transportModes.forEach(async (mode) => {
await cases.forEach(async ({ title, options }) => {
title += ` (${
Object.keys(mode).length ? mode.transportMode : 'default'
})`;
options = { ...mode, ...options };
const testOptions = Object.assign({}, baseOptions, options);

// TODO: use async/await when Node.js v6 support is dropped
Promise.resolve()
.then(() => {
return new Promise((resolve) => {
testServer.startAwaitingCompilation(config, testOptions, resolve);
});
})
.then(() => {
// make sure the previous Promise is not passing along strange arguments to runBrowser
return runBrowser();
})
.then(({ page, browser }) => {
return new Promise((resolve) => {
page.goto(`http://localhost:${port2}/main`);
page.on('console', ({ _text }) => {
res.push(_text);
});
// wait for load before closing the browser
page.waitForNavigation({ waitUntil: 'load' }).then(() => {
page.waitFor(beforeBrowserCloseDelay).then(() => {
browser.close().then(() => {
resolve();
});
});
});
});
})
.then(() => {
return new Promise((resolve) => {
testServer.close(resolve);
});
})
.then(() => {
// Order doesn't matter, maybe we should improve that in future
expect(res.sort()).toMatchSnapshot();
done();
await it(title, async (done) => {
await testServer.startAwaitingCompilation(config, testOptions);
const res = [];
const { page, browser } = await runBrowser();
page.goto(`http://localhost:${port2}/main`);
page.on('console', ({ _text }) => {
res.push(_text);
});
// wait for load before closing the browser
await page.waitForNavigation({ waitUntil: 'load' });
await page.waitFor(beforeBrowserCloseDelay);
await browser.close();
// Order doesn't matter, maybe we should improve that in future
await expect(res.sort()).toMatchSnapshot();
await testServer.close(done);
});
});
});
});
Loading