Skip to content

Commit 201920a

Browse files
snitin315snitin315
committed
refactor: avoid new public options on class
1 parent fc9028c commit 201920a

File tree

7 files changed

+109
-22
lines changed

7 files changed

+109
-22
lines changed

‎lib/Server.js

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -644,20 +644,20 @@ class Server {
644644
let networkUrlIPv4;
645645
let networkUrlIPv6;
646646

647-
if (this.hostname) {
648-
if (this.hostname === 'localhost') {
647+
if (this.options.host) {
648+
if (this.options.host === 'localhost') {
649649
localhost = prettyPrintUrl('localhost');
650650
} else {
651651
let isIP;
652652

653653
try {
654-
isIP = ipaddr.parse(this.hostname);
654+
isIP = ipaddr.parse(this.options.host);
655655
} catch (error) {
656656
// Ignore
657657
}
658658

659659
if (!isIP) {
660-
server = prettyPrintUrl(this.hostname);
660+
server = prettyPrintUrl(this.options.host);
661661
}
662662
}
663663
}
@@ -754,7 +754,7 @@ class Server {
754754
if (this.options.open) {
755755
const runOpen = require('./utils/runOpen');
756756

757-
const openTarget = prettyPrintUrl(this.hostname || 'localhost');
757+
const openTarget = prettyPrintUrl(this.options.host || 'localhost');
758758

759759
runOpen(openTarget, this.options.open, this.logger);
760760
}
@@ -763,29 +763,38 @@ class Server {
763763
listen(port, hostname, fn) {
764764
const findPort = require('./utils/findPort');
765765

766-
if (hostname === 'local-ip') {
767-
this.hostname = internalIp.v4.sync() || internalIp.v6.sync() || '0.0.0.0';
768-
} else if (hostname === 'local-ipv4') {
769-
this.hostname = internalIp.v4.sync() || '0.0.0.0';
770-
} else if (hostname === 'local-ipv6') {
771-
this.hostname = internalIp.v6.sync() || '::';
772-
} else {
773-
this.hostname = hostname;
766+
if (typeof hostname !== 'undefined' && hostname !== this.options.host) {
767+
throw new Error(
768+
'The host specified in options is different from the host passed as an argument.'
769+
);
774770
}
775771

776772
if (typeof port !== 'undefined' && port !== this.options.port) {
777773
this.logger.warn(
778-
'The port specified in options and the port passed as an argument is different.'
774+
'The port specified in options is different from the port passed as an argument.'
779775
);
780776
}
781777

778+
hostname = this.options.host;
779+
780+
if (hostname === 'local-ip') {
781+
this.options.host =
782+
internalIp.v4.sync() || internalIp.v6.sync() || '0.0.0.0';
783+
} else if (hostname === 'local-ipv4') {
784+
this.options.host = internalIp.v4.sync() || '0.0.0.0';
785+
} else if (hostname === 'local-ipv6') {
786+
this.options.host = internalIp.v6.sync() || '::';
787+
} else {
788+
this.options.host = hostname;
789+
}
790+
782791
return (
783792
findPort(port || this.options.port)
784793
// eslint-disable-next-line no-shadow
785794
.then((port) => {
786795
this.options.port = port;
787796

788-
return this.server.listen(port, this.hostname, (error) => {
797+
return this.server.listen(port, this.options.host, (error) => {
789798
if (this.options.hot || this.options.liveReload) {
790799
this.createSocketServer();
791800
}
@@ -923,12 +932,12 @@ class Server {
923932
// these are removed from the hostname in url.parse(),
924933
// so we have the pure IPv6-address in hostname.
925934
// always allow localhost host, for convenience (hostname === 'localhost')
926-
// allow hostname of listening address (hostname === this.hostname)
935+
// allow hostname of listening address (hostname === this.options.host)
927936
const isValidHostname =
928937
ipaddr.IPv4.isValid(hostname) ||
929938
ipaddr.IPv6.isValid(hostname) ||
930939
hostname === 'localhost' ||
931-
hostname === this.hostname;
940+
hostname === this.options.host;
932941

933942
if (isValidHostname) {
934943
return true;

‎test/helpers/test-server.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,8 @@ function startFullSetup(config, options, done) {
2727
console.warn('Using the default port for testing is not recommended');
2828
port = 8080;
2929
}
30-
const host = Object.prototype.hasOwnProperty.call(options, 'host')
31-
? options.host
32-
: 'localhost';
3330

34-
server.listen(port, host, (err) => {
31+
server.listen(port, options.host, (err) => {
3532
if (err && done) {
3633
return done(err);
3734
}

‎test/server/Server.test.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ jest.mock('sockjs/lib/transport');
1212

1313
const baseDevConfig = {
1414
port,
15+
host: 'localhost',
1516
static: false,
1617
};
1718

@@ -157,6 +158,52 @@ describe('Server', () => {
157158
});
158159
});
159160

161+
describe('listen', () => {
162+
let compiler;
163+
let server;
164+
165+
beforeAll(() => {
166+
compiler = webpack(config);
167+
});
168+
169+
it('should listen and not throw error', (done) => {
170+
const options = {
171+
host: 'localhost',
172+
port,
173+
};
174+
175+
server = new Server(compiler, options);
176+
177+
expect(() => server.listen(port, 'localhost')).not.toThrowError();
178+
server.close(done);
179+
});
180+
181+
it('should work and listen', (done) => {
182+
const options = {
183+
host: 'localhost',
184+
};
185+
186+
server = new Server(compiler, options);
187+
188+
expect(() => server.listen(9000)).not.toThrowError();
189+
server.close(done);
190+
});
191+
192+
it('should throw an error', (done) => {
193+
const options = {
194+
host: '192.160.21.34',
195+
port,
196+
};
197+
198+
server = new Server(compiler, options);
199+
200+
expect(() => server.listen(port, 'localhost')).toThrowError(
201+
'The host specified in options is different from the host passed as an argument.'
202+
);
203+
server.close(done);
204+
});
205+
});
206+
160207
describe('checkHost', () => {
161208
let compiler;
162209
let server;

‎test/server/host-option.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('host option', () => {
3535
it('server address', () => {
3636
const address = server.server.address();
3737

38-
expect(address.address).toBe('127.0.0.1');
38+
expect(address.address).toBe('::');
3939
expect(address.port).toBe(port);
4040
});
4141

‎test/server/open-option.test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ describe('"open" option', () => {
7575
const server = createServer(compiler, {
7676
open: true,
7777
port,
78+
host: '0.0.0.0',
7879
static: false,
7980
});
8081

@@ -97,6 +98,7 @@ describe('"open" option', () => {
9798
const server = createServer(compiler, {
9899
open: true,
99100
port,
101+
host: '::',
100102
static: false,
101103
});
102104

@@ -119,6 +121,7 @@ describe('"open" option', () => {
119121
const server = createServer(compiler, {
120122
open: true,
121123
port,
124+
host: 'localhost',
122125
static: false,
123126
});
124127

@@ -141,6 +144,7 @@ describe('"open" option', () => {
141144
const server = createServer(compiler, {
142145
open: true,
143146
port,
147+
host: '127.0.0.1',
144148
static: false,
145149
});
146150

@@ -163,6 +167,7 @@ describe('"open" option', () => {
163167
const server = createServer(compiler, {
164168
open: true,
165169
port,
170+
host: '::1',
166171
static: false,
167172
});
168173

@@ -185,6 +190,7 @@ describe('"open" option', () => {
185190
const server = createServer(compiler, {
186191
open: true,
187192
port,
193+
host: internalIPv4,
188194
static: false,
189195
});
190196

@@ -232,6 +238,7 @@ describe('"open" option', () => {
232238
const server = createServer(compiler, {
233239
open: true,
234240
port,
241+
host: 'localhost',
235242
static: false,
236243
});
237244

@@ -254,6 +261,7 @@ describe('"open" option', () => {
254261
const server = createServer(compiler, {
255262
open: false,
256263
port,
264+
host: 'localhost',
257265
static: false,
258266
});
259267

@@ -274,6 +282,7 @@ describe('"open" option', () => {
274282
const server = createServer(compiler, {
275283
open: 'index.html',
276284
port,
285+
host: 'localhost',
277286
static: false,
278287
});
279288

@@ -296,6 +305,7 @@ describe('"open" option', () => {
296305
const server = createServer(compiler, {
297306
open: '/index.html',
298307
port,
308+
host: 'localhost',
299309
static: false,
300310
});
301311

@@ -318,6 +328,7 @@ describe('"open" option', () => {
318328
const server = createServer(compiler, {
319329
open: 'http://localhost:8117/index.html',
320330
port,
331+
host: 'localhost',
321332
static: false,
322333
});
323334

@@ -340,6 +351,7 @@ describe('"open" option', () => {
340351
const server = createServer(compiler, {
341352
open: ['first.html', 'second.html'],
342353
port,
354+
host: 'localhost',
343355
static: false,
344356
});
345357

@@ -376,6 +388,7 @@ describe('"open" option', () => {
376388
'http://localhost:8117/second.html',
377389
],
378390
port,
391+
host: 'localhost',
379392
static: false,
380393
});
381394

@@ -409,6 +422,7 @@ describe('"open" option', () => {
409422
const server = createServer(compiler, {
410423
open: {},
411424
port,
425+
host: 'localhost',
412426
static: false,
413427
});
414428

@@ -433,6 +447,7 @@ describe('"open" option', () => {
433447
target: true,
434448
},
435449
port,
450+
host: 'localhost',
436451
static: false,
437452
});
438453

@@ -457,6 +472,7 @@ describe('"open" option', () => {
457472
target: 'index.html',
458473
},
459474
port,
475+
host: 'localhost',
460476
static: false,
461477
});
462478

@@ -481,6 +497,7 @@ describe('"open" option', () => {
481497
target: ['first.html', 'second.html'],
482498
},
483499
port,
500+
host: 'localhost',
484501
static: false,
485502
});
486503

@@ -516,6 +533,7 @@ describe('"open" option', () => {
516533
app: 'google-chrome',
517534
},
518535
port,
536+
host: 'localhost',
519537
static: false,
520538
});
521539

@@ -541,6 +559,7 @@ describe('"open" option', () => {
541559
app: ['google-chrome', '--incognito'],
542560
},
543561
port,
562+
host: 'localhost',
544563
static: false,
545564
});
546565

@@ -567,6 +586,7 @@ describe('"open" option', () => {
567586
app: 'google-chrome',
568587
},
569588
port,
589+
host: 'localhost',
570590
static: false,
571591
});
572592

@@ -593,6 +613,7 @@ describe('"open" option', () => {
593613
app: ['google-chrome', '--incognito'],
594614
},
595615
port,
616+
host: 'localhost',
596617
static: false,
597618
});
598619

@@ -631,6 +652,7 @@ describe('"open" option', () => {
631652
app: ['google-chrome', '--incognito'],
632653
},
633654
port,
655+
host: 'localhost',
634656
static: false,
635657
});
636658

@@ -668,6 +690,7 @@ describe('"open" option', () => {
668690
const server = createServer(compiler, {
669691
open: true,
670692
port,
693+
host: 'localhost',
671694
static: false,
672695
});
673696
const loggerWarnSpy = jest.spyOn(server.logger, 'warn');

0 commit comments

Comments
 (0)