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

fix: empty entries #2920

Merged
merged 4 commits into from
Feb 17, 2021
Merged

fix: empty entries #2920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3888b32
fix: empty entries
alexander-akait Dec 8, 2020
4f646db
test: refactor
snitin315 Feb 17, 2021
a224617
test: refactor
snitin315 Feb 17, 2021
a6fa2c5
test: udpates
snitin315 Feb 17, 2021
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
44 changes: 23 additions & 21 deletions lib/utils/DevServerPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class DevServerPlugin {
: '';
/** @type {string} */
let path = '';

// only add the path if it is not default
if (
options.client &&
Expand All @@ -45,11 +46,13 @@ class DevServerPlugin {
) {
path = `&path=${options.client.path}`;
}

/** @type {string} */
const port =
options.client && options.client.port
? `&port=${options.client.port}`
: '';

/** @type {string} */
const clientEntry = `${require.resolve(
'../../client/default/'
Expand Down Expand Up @@ -119,14 +122,22 @@ class DevServerPlugin {
*/
// eslint-disable-next-line no-shadow
const checkInject = (option, _config, defaultValue) => {
if (typeof option === 'boolean') return option;
if (typeof option === 'function') return option(_config);
if (typeof option === 'boolean') {
return option;
}

if (typeof option === 'function') {
return option(_config);
}

return defaultValue;
};

const compilerOptions = compiler.options;

compilerOptions.plugins = compilerOptions.plugins || [];
/** @type {Boolean} */

/** @type {boolean} */
const isWebTarget = compilerOptions.externalsPresets
? compilerOptions.externalsPresets.web
: [
Expand All @@ -138,6 +149,7 @@ class DevServerPlugin {
undefined,
null,
].includes(compilerOptions.target);

/** @type {Entry} */
const additionalEntries = checkInject(
options.injectClient,
Expand All @@ -153,24 +165,12 @@ class DevServerPlugin {

// use a hook to add entries if available
if (EntryPlugin) {
compiler.hooks.make.tapPromise('DevServerPlugin', (compilation) =>
Promise.all(
additionalEntries.map(
(entry) =>
new Promise((resolve, reject) => {
compilation.addEntry(
compiler.context,
EntryPlugin.createDependency(entry, {}),
{}, // global entry
(err) => {
if (err) return reject(err);
resolve();
}
);
})
)
)
);
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
for (const additionalEntry of additionalEntries) {
new EntryPlugin(compiler.context, additionalEntry, {
// eslint-disable-next-line no-undefined
name: undefined,
}).apply(compiler);
}
} else {
compilerOptions.entry = prependEntry(
compilerOptions.entry || './src',
Expand All @@ -185,6 +185,7 @@ class DevServerPlugin {
const providePlugin = new webpack.ProvidePlugin({
__webpack_dev_server_client__: getSocketClientPath(options),
});

providePlugin.apply(compiler);

if (
Expand All @@ -195,6 +196,7 @@ class DevServerPlugin {
) {
// apply the HMR plugin, if it didn't exist before.
const plugin = new webpack.HotModuleReplacementPlugin();

plugin.apply(compiler);
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/utils/updateCompiler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const DevServerPlugin = require('./DevServerPlugin');

function updateCompiler(compiler, options) {
const compilers = compiler.compilers || [compiler];

// eslint-disable-next-line no-shadow
compilers.forEach((compiler) => {
new DevServerPlugin(options).apply(compiler);
Expand Down
114 changes: 114 additions & 0 deletions test/e2e/DevServer.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
'use strict';

const testBin = require('../helpers/test-bin');
const isWebpack5 = require('../helpers/isWebpack5');

describe('DevServer', () => {
const webpack5Test = isWebpack5 ? it : it.skip;
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

it('should add devServer entry points to a single entry point', (done) => {
testBin('--config ./test/fixtures/dev-server/default-config.js')
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('client/default/index.js?');
done();
})
.catch(done);
});

it('should add devServer entry points to a multi entry point object', (done) => {
testBin(
'--config ./test/fixtures/dev-server/multi-entry.js --stats=verbose'
)
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('client/default/index.js?');
expect(output.stderr).toContain('foo.js');
done();
})
.catch(done);
});

webpack5Test('should supports entry as descriptor', (done) => {
testBin(
'--config ./test/fixtures/entry-as-descriptor/webpack.config --stats detailed'
)
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('foo.js');
done();
})
.catch(done);
});

it('should only prepends devServer entry points to "web" target', (done) => {
testBin(
'--config ./test/fixtures/dev-server/default-config.js --target web'
)
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('client/default/index.js?');
expect(output.stderr).toContain('foo.js');
done();
})
.catch(done);
});

it('should not prepend devServer entry points to "node" target', (done) => {
testBin(
'--config ./test/fixtures/dev-server/default-config.js --target node'
)
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).not.toContain('client/default/index.js?');
expect(output.stderr).toContain('foo.js');
done();
})
.catch(done);
});

it('should prepends the hot runtime to "node" target as well', (done) => {
testBin(
'--config ./test/fixtures/dev-server/default-config.js --target node --hot'
)
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('webpack/hot/dev-server');
done();
})
.catch(done);
});

it('does not use client.path when default', (done) => {
testBin('--config ./test/fixtures/dev-server/client-default-path-config.js')
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).not.toContain('&path=/ws');
done();
})
.catch(done);
});

it('should use client.path when custom', (done) => {
testBin('--config ./test/fixtures/dev-server/client-custom-path-config.js')
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('&path=/custom/path');
done();
})
.catch(done);
});

webpack5Test(
'should prepend devServer entry points depending on targetProperties',
(done) => {
testBin('--config ./test/fixtures/dev-server/target-config.js')
.then((output) => {
expect(output.exitCode).toEqual(0);
expect(output.stderr).toContain('client/default/index.js');
done();
})
.catch(done);
}
);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

In future we should rewrite these tests on e2e with browser, i.e. run dev server, do some action and wait HMR or reload refresh using puppeter or playwright, we can do it in separate PR, should be interesting task

3 changes: 3 additions & 0 deletions test/fixtures/dev-server/bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict';

console.log('I am bar');
18 changes: 18 additions & 0 deletions test/fixtures/dev-server/client-custom-path-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const { resolve } = require('path');

module.exports = {
mode: 'development',
stats: 'detailed',
entry: resolve(__dirname, './foo.js'),
devServer: {
client: {
path: '/custom/path',
},
transportMode: {
server: 'sockjs',
client: 'sockjs',
},
},
};
18 changes: 18 additions & 0 deletions test/fixtures/dev-server/client-default-path-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const { resolve } = require('path');

module.exports = {
mode: 'development',
stats: 'detailed',
entry: resolve(__dirname, './foo.js'),
devServer: {
client: {
path: '/ws',
},
transportMode: {
server: 'sockjs',
client: 'sockjs',
},
},
};
18 changes: 18 additions & 0 deletions test/fixtures/dev-server/default-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const { resolve } = require('path');

module.exports = {
mode: 'development',
stats: 'detailed',
entry: resolve(__dirname, './foo.js'),
devServer: {
client: {
path: '/custom/path',
},
transportMode: {
server: 'sockjs',
client: 'sockjs',
},
},
};
3 changes: 3 additions & 0 deletions test/fixtures/dev-server/foo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict';

console.log('I am foo');
19 changes: 19 additions & 0 deletions test/fixtures/dev-server/multi-entry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

const { resolve } = require('path');

module.exports = {
mode: 'development',
stats: 'detailed',
context: __dirname,
entry: {
foo: resolve(__dirname, './foo.js'),
bar: resolve(__dirname, './bar.js'),
},
devServer: {
transportMode: {
server: 'sockjs',
client: 'sockjs',
},
},
};
24 changes: 24 additions & 0 deletions test/fixtures/dev-server/target-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const { resolve } = require('path');

module.exports = {
mode: 'development',
stats: 'detailed',
entry: resolve(__dirname, './foo.js'),
target: ['web', 'webworker'],
output: {
chunkLoading: false,
wasmLoading: false,
workerChunkLoading: false,
},
devServer: {
client: {
path: '/custom/path',
},
transportMode: {
server: 'sockjs',
client: 'sockjs',
},
},
};