Skip to content

Commit bb16d44

Browse files
fix: do not apply HotModuleReplacement plugin twice (#2269)
1 parent 1dae54d commit bb16d44

File tree

5 files changed

+63
-51
lines changed

5 files changed

+63
-51
lines changed

packages/serve/src/index.ts

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,26 +49,23 @@ class ServeCommand {
4949
const processors: Array<(opts: Record<string, any>) => void> = [];
5050

5151
for (const optionName in options) {
52-
if (optionName === 'hot') {
53-
devServerOptions[optionName] = options[optionName];
52+
const kebabedOption = cli.utils.toKebabCase(optionName);
53+
// `webpack-dev-server` has own logic for the `--hot` option
54+
const isBuiltInOption =
55+
kebabedOption !== 'hot' && builtInOptions.find((builtInOption) => builtInOption.name === kebabedOption);
56+
57+
if (isBuiltInOption) {
5458
webpackOptions[optionName] = options[optionName];
5559
} else {
56-
const kebabedOption = cli.utils.toKebabCase(optionName);
57-
const isBuiltInOption = builtInOptions.find((builtInOption) => builtInOption.name === kebabedOption);
58-
59-
if (isBuiltInOption) {
60-
webpackOptions[optionName] = options[optionName];
61-
} else {
62-
const needToProcess = devServerFlags.find(
63-
(devServerOption) => devServerOption.name === kebabedOption && devServerOption.processor,
64-
);
60+
const needToProcess = devServerFlags.find(
61+
(devServerOption) => devServerOption.name === kebabedOption && devServerOption.processor,
62+
);
6563

66-
if (needToProcess) {
67-
processors.push(needToProcess.processor);
68-
}
69-
70-
devServerOptions[optionName] = options[optionName];
64+
if (needToProcess) {
65+
processors.push(needToProcess.processor);
7166
}
67+
68+
devServerOptions[optionName] = options[optionName];
7269
}
7370
}
7471

test/serve/basic/serve-basic.test.js

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ describe('basic serve usage', () => {
2727
}
2828

2929
it('should work', async () => {
30-
const { stderr, stdout } = await runServe(['--no-hot'], __dirname);
30+
const { stderr, stdout } = await runServe([''], __dirname);
3131

3232
expect(stderr).toBeFalsy();
3333
expect(stdout).toContain('main.js');
34-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
34+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
3535
});
3636

3737
it('should work with the "--mode" option', async () => {
@@ -40,7 +40,7 @@ describe('basic serve usage', () => {
4040
expect(stderr).toBeFalsy();
4141
expect(stdout).toContain('development');
4242
expect(stdout).toContain('main.js');
43-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
43+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
4444
});
4545

4646
it('should work with the "--mode" option #2', async () => {
@@ -49,99 +49,99 @@ describe('basic serve usage', () => {
4949
expect(stderr).toBeFalsy();
5050
expect(stdout).toContain('production');
5151
expect(stdout).toContain('main.js');
52-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
52+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
5353
});
5454

55-
it('should work with the "--mode" option #2', async () => {
55+
it('should work with the "--mode" option #3', async () => {
5656
const { stderr, stdout } = await runServe(['--mode', 'development'], __dirname);
5757

5858
expect(stderr).toBeFalsy();
5959
expect(stdout).toContain('development');
6060
expect(stdout).toContain('main.js');
61-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
61+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
6262
});
6363

6464
it('should work with the "--progress" option', async () => {
6565
const { stderr, stdout } = await runServe(['--progress'], __dirname);
6666

6767
expect(stderr).toContain('webpack.Progress');
6868
expect(stdout).toContain('main.js');
69-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
69+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
7070
});
7171

7272
it('should work with the "--progress" option using the "profile" value', async () => {
7373
const { stderr, stdout } = await runServe(['--progress', 'profile'], __dirname);
7474

7575
expect(stderr).toContain('webpack.Progress');
7676
expect(stdout).toContain('main.js');
77-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
77+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
7878
});
7979

80-
it('should work with flags', async () => {
81-
const { stderr, stdout } = await runServe(['--hot'], __dirname);
82-
83-
expect(stderr).toBeFalsy();
84-
expect(stdout).toContain('main.js');
85-
expect(stdout).toContain('HotModuleReplacementPlugin');
86-
});
87-
88-
it('should respect the --no-color flag', async () => {
80+
it('should log help information and respect the "--no-color" option', async () => {
8981
const { stdout, stderr } = await runServe(['--help', '--no-color'], __dirname);
9082

9183
expect(stderr).toBeFalsy();
9284
expect(stdout).toContain(usageText);
9385
expect(stdout).toContain(descriptionText);
9486
});
9587

96-
it('should not invoke info subcommand', async () => {
88+
it('should work with the "--client-log-level" option', async () => {
9789
const { stdout, stderr } = await runServe(['--client-log-level', 'info'], testPath);
9890

9991
expect(stderr).toBeFalsy();
10092
expect(stdout).toContain('main.js');
101-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
93+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
10294
});
10395

104-
it('compiles without flags', async () => {
96+
it('should work with the "--port" option', async () => {
10597
const { stdout, stderr } = await runServe(['--port', port], testPath);
10698

10799
expect(stderr).toBeFalsy();
108100
expect(stdout).toContain('main.js');
109-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
101+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
110102
});
111103

112-
it('uses hot flag to alter bundle', async () => {
113-
const { stdout, stderr } = await runServe(['--port', port, '--hot'], testPath);
104+
it('should work with the "--hot" option', async () => {
105+
const { stderr, stdout } = await runServe(['--hot'], __dirname);
114106

115107
expect(stderr).toBeFalsy();
116108
expect(stdout).toContain('main.js');
117-
expect(stdout).toContain('HotModuleReplacementPlugin');
109+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toHaveLength(1);
118110
});
119111

120-
it('uses hot-only flag to alter bundle', async () => {
112+
it('should work with the "--no-hot" option', async () => {
113+
const { stdout, stderr } = await runServe(['--port', port, '--no-hot'], testPath);
114+
115+
expect(stderr).toBeFalsy();
116+
expect(stdout).toContain('main.js');
117+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toBeNull();
118+
});
119+
120+
it('should work with the "--hot" option using the "only" value', async () => {
121121
const { stdout, stderr } = await runServe(['--port', port, isDevServer4 ? '--hot only' : '--hot-only'], testPath);
122122

123123
expect(stderr).toBeFalsy();
124124
expect(stdout).toContain('main.js');
125-
expect(stdout).toContain('HotModuleReplacementPlugin');
125+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toHaveLength(1);
126126
});
127127

128-
it('uses no-hot flag', async () => {
129-
const { stdout, stderr } = await runServe(['--port', port, '--no-hot'], testPath);
128+
it('should work with "--hot" and "--port" options', async () => {
129+
const { stdout, stderr } = await runServe(['--port', port, '--hot'], testPath);
130130

131131
expect(stderr).toBeFalsy();
132132
expect(stdout).toContain('main.js');
133-
expect(stdout).not.toContain('HotModuleReplacementPlugin');
133+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toHaveLength(1);
134134
});
135135

136-
it('uses hot flag and progress flag', async () => {
136+
it('should work with the "--hot" and "--progress" options', async () => {
137137
const { stdout, stderr } = await runServe(['--port', port, '--hot', '--progress'], testPath);
138138

139139
expect(stderr).toContain('webpack.Progress');
140140
expect(stdout).toContain('main.js');
141-
expect(stdout).toContain('HotModuleReplacementPlugin');
141+
expect(stdout.match(/HotModuleReplacementPlugin/g)).toHaveLength(1);
142142
});
143143

144-
it('throws error on unknown flag', async () => {
144+
it('should log and error on unknown flag', async () => {
145145
const { exitCode, stdout, stderr } = await runServe(['--port', port, '--unknown-flag'], testPath);
146146

147147
expect(exitCode).toBe(2);

test/serve/basic/webpack.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ const WebpackCLITestPlugin = require('../../utils/webpack-cli-test-plugin');
33
module.exports = {
44
mode: 'development',
55
devtool: false,
6-
plugins: [new WebpackCLITestPlugin(['mode', 'plugins'], false)],
6+
plugins: [new WebpackCLITestPlugin(['mode'], false, 'hooks.compilation.taps')],
77
};

test/utils/cli-plugin-test/plugin.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,6 @@ describe('webpack-cli-test-plugin Test', () => {
1414
expect(stdout).toContain(`alias: { alias: [ 'alias1', 'alias2' ] }`);
1515
}
1616

17-
expect(stdout).toContain(` WebpackCLITestPlugin { opts: [Array], showAll: true }`);
17+
expect(stdout).toContain('WebpackCLITestPlugin');
1818
});
1919
});

test/utils/webpack-cli-test-plugin.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,32 @@
11
class WebpackCLITestPlugin {
2-
constructor(options, showAll = true) {
2+
constructor(options, showAll = true, showHooks) {
33
this.opts = options;
44
this.showAll = showAll;
5+
this.showHooks = showHooks;
56
}
67

78
apply(compiler) {
89
compiler.hooks.done.tap('webpack-cli Test Plugin', () => {
10+
if (this.showHooks) {
11+
const identifiers = this.showHooks.split('.');
12+
13+
let shown = compiler;
14+
15+
identifiers.forEach((identifier) => {
16+
shown = shown[identifier];
17+
});
18+
19+
console.log(shown);
20+
}
21+
922
if (this.showAll) {
1023
console.log(compiler.options);
1124
}
25+
1226
if (this.opts) {
1327
this.opts.map((e) => {
1428
const config = Object.getOwnPropertyDescriptor(compiler.options, e);
29+
1530
console.log(config.value);
1631
});
1732
}

0 commit comments

Comments
 (0)