Skip to content

fix: apply client logging options directly on client loading #2946

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

Closed
wants to merge 1 commit into from
Closed

fix: apply client logging options directly on client loading #2946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 7 additions & 11 deletions client-src/default/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const overlay = require('./overlay');
const { log, setLogLevel } = require('./utils/log');
const sendMessage = require('./utils/sendMessage');
const reloadApp = require('./utils/reloadApp');
const getUrlOptions = require('./utils/getUrlOptions');
const createSocketUrl = require('./utils/createSocketUrl');

const status = {
Expand All @@ -27,12 +28,17 @@ const options = {
useErrorOverlay: false,
useProgress: false,
};
const socketUrl = createSocketUrl(__resourceQuery);
const urlOptions = getUrlOptions(__resourceQuery);
const socketUrl = createSocketUrl(urlOptions);

self.addEventListener('beforeunload', () => {
status.isUnloading = true;
});

if (urlOptions.query.logging) {
setLogLevel(urlOptions.query.logging);
}

if (typeof window !== 'undefined') {
const qs = window.location.search.toLowerCase();
options.hotReload = qs.indexOf('hotreload=false') === -1;
Expand Down Expand Up @@ -65,16 +71,6 @@ const onSocketMessage = {
}
sendMessage('StillOk');
},
logging: function logging(level) {
// this is needed because the HMR logger operate separately from
// dev server logger
const hotCtx = require.context('webpack/hot', false, /^\.\/log$/);
if (hotCtx.keys().indexOf('./log') !== -1) {
hotCtx('./log').setLogLevel(level);
}

setLogLevel(level);
},
overlay(value) {
if (typeof document !== 'undefined') {
if (typeof value === 'boolean') {
Expand Down
40 changes: 8 additions & 32 deletions client-src/default/utils/createSocketUrl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,19 @@
/* global self */

const url = require('url');
const getCurrentScriptSource = require('./getCurrentScriptSource');

function createSocketUrl(resourceQuery, currentLocation) {
let urlParts;

if (typeof resourceQuery === 'string' && resourceQuery !== '') {
// If this bundle is inlined, use the resource query to get the correct url.
// format is like `?http://0.0.0.0:8096&port=8097&host=localhost`
urlParts = url.parse(
resourceQuery
// strip leading `?` from query string to get a valid URL
.substr(1)
// replace first `&` with `?` to have a valid query string
.replace('&', '?'),
true
);
} else {
// Else, get the url from the <script> this file was called with.
const scriptHost = getCurrentScriptSource();
urlParts = url.parse(scriptHost || '/', true, true);
}

// Use parameter to allow passing location in unit tests
if (typeof currentLocation === 'string' && currentLocation !== '') {
currentLocation = url.parse(currentLocation);
} else {
currentLocation = self.location;
}

return getSocketUrl(urlParts, currentLocation);
}

/*
* Gets socket URL based on Script Source/Location
* (scriptSrc: URL, location: URL) -> URL
*/
function getSocketUrl(urlParts, loc) {
function createSocketUrl(urlParts, loc) {
// Use parameter to allow passing location in unit tests
if (typeof loc === 'string' && loc !== '') {
Copy link
Member

Choose a reason for hiding this comment

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

Is it only for tests?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, it is. I kept it how it was before. (I've tried removing it and overriding self.location in the test but that didn't work. Probably because self doesn't work as expected in the test environment?)

Copy link
Member

Choose a reason for hiding this comment

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

hm, weird, self should work, jest allows to mock this

Copy link
Author

Choose a reason for hiding this comment

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

I guess that's why it has already been mocked before

Copy link
Member

Choose a reason for hiding this comment

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

I think we should avoid code for testing from runtime...

loc = url.parse(loc);
} else {
loc = self.location;
}

const { auth, query } = urlParts;
let { hostname, protocol, port } = urlParts;

Expand Down
31 changes: 31 additions & 0 deletions client-src/default/utils/getUrlOptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

/* global self */

const url = require('url');
const getCurrentScriptSource = require('./getCurrentScriptSource');

function getUrlOptions(resourceQuery) {
let options;

if (typeof resourceQuery === 'string' && resourceQuery !== '') {
// If this bundle is inlined, use the resource query to get the correct url.
// format is like `?http://0.0.0.0:8096&port=8097&host=localhost`
options = url.parse(
resourceQuery
// strip leading `?` from query string to get a valid URL
.substr(1)
// replace first `&` with `?` to have a valid query string
.replace('&', '?'),
true
);
} else {
// Else, get the url from the <script> this file was called with.
const scriptHost = getCurrentScriptSource();
options = url.parse(scriptHost || '/', true, true);
}

return options;
}

module.exports = getUrlOptions;
2 changes: 2 additions & 0 deletions client-src/default/utils/log.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const webpackHotLog = require('webpack/hot/log');
const log = require('../../transpiled-modules/log');

const name = 'webpack-dev-server';
Expand All @@ -8,6 +9,7 @@ const name = 'webpack-dev-server';
const defaultLevel = 'info';

function setLogLevel(level) {
webpackHotLog.setLogLevel(level);
log.configureDefaultLogger({
level,
});
Expand Down
4 changes: 0 additions & 4 deletions lib/Server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,10 +556,6 @@ class Server {
}
});

if (this.options.client.logging) {
this.sockWrite([connection], 'logging', this.options.client.logging);
}

if (this.options.hot === true || this.options.hot === 'only') {
this.sockWrite([connection], 'hot');
}
Expand Down
6 changes: 5 additions & 1 deletion lib/utils/DevServerPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,14 @@ class DevServerPlugin {
options.client && options.client.port
? `&port=${options.client.port}`
: '';
const logging =
options.client && options.client.logging
? `&logging=${options.client.logging}`
: '';
/** @type {string} */
const clientEntry = `${require.resolve(
'../../client/default/'
)}?${domain}${host}${path}${port}`;
)}?${domain}${host}${path}${port}${logging}`;

/** @type {(string[] | string)} */
let hotEntry;
Expand Down
34 changes: 21 additions & 13 deletions test/cli/__snapshots__/cli.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ exports[`CLI --hot webpack 4 1`] = `
<i> [../../../client/default/overlay.js] Xdir/client/default/overlay.js X KiB {main} [built]
<i> [../../../client/default/socket.js] Xdir/client/default/socket.js X KiB {main} [built]
<i> [../../../client/default/utils/createSocketUrl.js] Xdir/client/default/utils/createSocketUrl.js X KiB {main} [built]
<i> [../../../client/default/utils/getUrlOptions.js] Xdir/client/default/utils/getUrlOptions.js X bytes {main} [built]
<i> [../../../client/default/utils/log.js] Xdir/client/default/utils/log.js X bytes {main} [built]
<i> [../../../client/default/utils/reloadApp.js] Xdir/client/default/utils/reloadApp.js X KiB {main} [built]
<i> [../../../client/default/utils/sendMessage.js] Xdir/client/default/utils/sendMessage.js X bytes {main} [built]
<i> [../../../client/transpiled-modules/strip-ansi.js] Xdir/client/transpiled-modules/strip-ansi.js X bytes {main} [built]
<i> [../../../node_modules/webpack/hot sync ^\\\\.\\\\/log$] (webpack)/hot sync nonrecursive ^\\\\.\\\\/log$ X bytes {main} [built]
<i> [../../../node_modules/webpack/hot/dev-server.js] (webpack)/hot/dev-server.js X KiB {main} [built]
<i> [../../../node_modules/webpack/hot/emitter.js] (webpack)/hot/emitter.js X bytes {main} [built]
<i> [../../../node_modules/webpack/hot/log-apply-result.js] (webpack)/hot/log-apply-result.js X KiB {main} [built]
Expand All @@ -33,16 +33,20 @@ exports[`CLI --hot webpack 5 1`] = `
<i> [webpack-dev-server] Content not from webpack x.x.x served from 'Xdir/static' directory
<i> [webpack-dev-middleware] asset main.js X KiB [emitted] (name: main)
<i> runtime modules X KiB 10 modules
<i> cacheable modules X KiB
<i> modules by path ../../../node_modules/ X KiB 16 modules
<i> modules by path ../../../ X KiB
<i> modules by path ../../../node_modules/ X KiB
<i> modules by path ../../../node_modules/webpack/hot/*.js X KiB 4 modules
<i> modules by path ../../../node_modules/html-entities/lib/*.js X KiB 4 modules
<i> modules by path ../../../node_modules/url/ X KiB 3 modules
<i> modules by path ../../../node_modules/querystring/*.js X KiB 3 modules
<i> 2 modules
<i> modules by path ../../../client/ X KiB
<i> modules by path ../../../client/default/ X KiB 8 modules
<i> modules by path ../../../client/default/ X KiB 9 modules
<i> modules by path ../../../client/transpiled-modules/*.js X KiB 2 modules
<i> modules by path ../../../client/clients/*.js X KiB
<i> ../../../client/clients/WebsocketClient.js X KiB [built] [code generated]
<i> ../../../client/clients/BaseClient.js X KiB [built] [code generated]
<i> ./foo.js X bytes [built] [code generated]
<i> ../../../node_modules/webpack/hot/ sync nonrecursive ^\\\\.\\\\/log$ X bytes [built] [code generated]
<i> ./foo.js X bytes [built] [code generated]
<i> webpack x.x.x compiled successfully in X ms
<i> [webpack-dev-middleware] Compiled successfully."
`;
Expand All @@ -63,13 +67,13 @@ exports[`CLI --no-hot webpack 4 1`] = `
<i> [../../../client/default/socket.js] Xdir/client/default/socket.js X KiB {main} [built]
<i> [../../../client/default/utils/createSocketUrl.js] Xdir/client/default/utils/createSocketUrl.js X KiB {main} [built]
<i> [../../../client/default/utils/getCurrentScriptSource.js] Xdir/client/default/utils/getCurrentScriptSource.js X bytes {main} [built]
<i> [../../../client/default/utils/getUrlOptions.js] Xdir/client/default/utils/getUrlOptions.js X bytes {main} [built]
<i> [../../../client/default/utils/log.js] Xdir/client/default/utils/log.js X bytes {main} [built]
<i> [../../../client/default/utils/reloadApp.js] Xdir/client/default/utils/reloadApp.js X KiB {main} [built]
<i> [../../../client/default/utils/sendMessage.js] Xdir/client/default/utils/sendMessage.js X bytes {main} [built]
<i> [../../../client/transpiled-modules/log.js] Xdir/client/transpiled-modules/log.js X KiB {main} [built]
<i> [../../../client/transpiled-modules/strip-ansi.js] Xdir/client/transpiled-modules/strip-ansi.js X bytes {main} [built]
<i> [../../../node_modules/ansi-html/index.js] Xdir/node_modules/ansi-html/index.js X KiB {main} [built]
<i> [../../../node_modules/webpack/hot sync ^\\\\.\\\\/log$] (webpack)/hot sync nonrecursive ^\\\\.\\\\/log$ X bytes {main} [built]
<i> [./foo.js] X bytes {main} [built]
<i> + 17 hidden modules
<i> [webpack-dev-middleware] Compiled successfully."
Expand All @@ -79,17 +83,21 @@ exports[`CLI --no-hot webpack 5 1`] = `
"<i> [webpack-dev-server] Project is running at http://localhost:8080/
<i> [webpack-dev-server] Content not from webpack x.x.x served from 'Xdir/static' directory
<i> [webpack-dev-middleware] asset main.js X KiB [emitted] (name: main)
<i> runtime modules X bytes 3 modules
<i> cacheable modules X KiB
<i> modules by path ../../../node_modules/ X KiB 14 modules
<i> runtime modules X bytes 2 modules
<i> modules by path ../../../ X KiB
<i> modules by path ../../../node_modules/ X KiB
<i> modules by path ../../../node_modules/html-entities/lib/*.js X KiB 4 modules
<i> modules by path ../../../node_modules/url/ X KiB 3 modules
<i> modules by path ../../../node_modules/querystring/*.js X KiB 3 modules
<i> modules by path ../../../node_modules/webpack/hot/*.js X KiB 2 modules
<i> 2 modules
<i> modules by path ../../../client/ X KiB
<i> modules by path ../../../client/default/ X KiB 8 modules
<i> modules by path ../../../client/default/ X KiB 9 modules
<i> modules by path ../../../client/transpiled-modules/*.js X KiB 2 modules
<i> modules by path ../../../client/clients/*.js X KiB
<i> ../../../client/clients/WebsocketClient.js X KiB [built] [code generated]
<i> ../../../client/clients/BaseClient.js X KiB [built] [code generated]
<i> ./foo.js X bytes [built] [code generated]
<i> ../../../node_modules/webpack/hot/ sync nonrecursive ^\\\\.\\\\/log$ X bytes [built] [code generated]
<i> ./foo.js X bytes [built] [code generated]
<i> webpack x.x.x compiled successfully in X ms
<i> [webpack-dev-middleware] Compiled successfully."
`;
3 changes: 2 additions & 1 deletion test/client/__snapshots__/index.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ Array [
"hot": [Function],
"invalid": [Function],
"liveReload": [Function],
"logging": [Function],
"ok": [Function],
"overlay": [Function],
"progress": [Function],
Expand All @@ -104,3 +103,5 @@ Array [
},
]
`;

exports[`index should update log level if options is passed 1`] = `"none"`;
19 changes: 14 additions & 5 deletions test/client/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('index', () => {
warn: jest.fn(),
error: jest.fn(),
},
setLogLevel: jest.fn(),
});
log = require('../../client-src/default/utils/log');

Expand All @@ -44,6 +45,15 @@ describe('index', () => {
jest.setMock('../../client-src/default/utils/sendMessage.js', jest.fn());
sendMessage = require('../../client-src/default/utils/sendMessage');

// getUrlOptions
jest.setMock('../../client-src/default/utils/getUrlOptions.js', () => {
return {
query: {
logging: 'none',
},
};
});

// createSocketUrl
jest.setMock(
'../../client-src/default/utils/createSocketUrl.js',
Expand Down Expand Up @@ -106,11 +116,6 @@ describe('index', () => {
expect(overlay.clear).toBeCalled();
});

// TODO: need to mock require.context
test.skip("should run onSocketMessage['logging']", () => {
onSocketMessage.logging();
});

test("should run onSocketMessage.progress and onSocketMessage['progress-update']", () => {
onSocketMessage.progress(false);
onSocketMessage['progress-update']({
Expand Down Expand Up @@ -235,4 +240,8 @@ describe('index', () => {
expect(log.log.error.mock.calls[0][0]).toMatchSnapshot();
expect(sendMessage.mock.calls[0][0]).toMatchSnapshot();
});

test('should update log level if options is passed', () => {
expect(log.setLogLevel.mock.calls[0][0]).toMatchSnapshot();
});
});
45 changes: 0 additions & 45 deletions test/client/utils/__snapshots__/createSocketUrl.test.js.snap
View file
Open in desktop

This file was deleted.

Loading