Skip to content

Commit a963ee2

Browse files
committed
fix(e2e): fix serverUrl port detection using URL constructor
The getServerUrl helper used .includes(':') to detect an explicit port, which false-positived on the scheme colon (e.g. http://localhost). Extract resolveServerUrl using the URL constructor and apply to both dev() and serve().
1 parent 33c0699 commit a963ee2

2 files changed

Lines changed: 78 additions & 14 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { resolveServerUrl } from '../application';
4+
5+
describe('resolveServerUrl', () => {
6+
describe('with opts.serverUrl', () => {
7+
it('appends port to a URL without an explicit port', () => {
8+
expect(resolveServerUrl('http://localhost', undefined, 3000)).toBe('http://localhost:3000');
9+
});
10+
11+
it('appends port to an https URL without an explicit port', () => {
12+
expect(resolveServerUrl('https://example.com', undefined, 4000)).toBe('https://example.com:4000');
13+
});
14+
15+
it('preserves an explicit port in the URL', () => {
16+
expect(resolveServerUrl('http://localhost:8080', undefined, 3000)).toBe('http://localhost:8080');
17+
});
18+
19+
it('handles a URL with a path (returns origin only)', () => {
20+
expect(resolveServerUrl('http://localhost/some/path', undefined, 3000)).toBe('http://localhost:3000');
21+
});
22+
23+
it('handles a bare hostname by appending port', () => {
24+
expect(resolveServerUrl('myhost', undefined, 5000)).toBe('myhost:5000');
25+
});
26+
27+
it('handles a bare IP address by appending port', () => {
28+
expect(resolveServerUrl('127.0.0.1', undefined, 5000)).toBe('127.0.0.1:5000');
29+
});
30+
});
31+
32+
describe('with fallback serverUrl', () => {
33+
it('uses fallback when opts.serverUrl is undefined', () => {
34+
expect(resolveServerUrl(undefined, 'http://fallback:9000', 3000)).toBe('http://fallback:9000');
35+
});
36+
37+
it('prefers opts.serverUrl over fallback', () => {
38+
expect(resolveServerUrl('http://localhost', 'http://fallback:9000', 3000)).toBe('http://localhost:3000');
39+
});
40+
});
41+
42+
describe('with no serverUrl at all', () => {
43+
it('defaults to http://localhost with the given port', () => {
44+
expect(resolveServerUrl(undefined, undefined, 4567)).toBe('http://localhost:4567');
45+
});
46+
47+
it('defaults when fallback is empty string', () => {
48+
expect(resolveServerUrl(undefined, '', 4567)).toBe('http://localhost:4567');
49+
});
50+
});
51+
});

integration/models/application.ts

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,31 @@ import type { EnvironmentConfig } from './environment.js';
99

1010
export type Application = ReturnType<typeof application>;
1111

12+
/**
13+
* Resolves the server URL for a dev/serve process, ensuring the runtime port
14+
* is always reflected in the URL. Uses the URL constructor to detect whether
15+
* an explicit port is present (avoiding false positives from the scheme colon).
16+
*/
17+
export const resolveServerUrl = (
18+
optsServerUrl: string | undefined,
19+
fallbackServerUrl: string | undefined,
20+
port: number,
21+
): string => {
22+
if (optsServerUrl) {
23+
try {
24+
const parsed = new URL(optsServerUrl);
25+
if (!parsed.port) {
26+
parsed.port = String(port);
27+
}
28+
return parsed.origin;
29+
} catch {
30+
// Bare host (e.g. "localhost"), not a full URL
31+
return `${optsServerUrl}:${port}`;
32+
}
33+
}
34+
return fallbackServerUrl || `http://localhost:${port}`;
35+
};
36+
1237
export const application = (
1338
config: ApplicationConfig,
1439
appDirPath: string,
@@ -58,13 +83,7 @@ export const application = (
5883
dev: async (opts: { port?: number; manualStart?: boolean; detached?: boolean; serverUrl?: string } = {}) => {
5984
const log = logger.child({ prefix: 'dev' }).info;
6085
const port = opts.port || (await getPort());
61-
const getServerUrl = () => {
62-
if (opts.serverUrl) {
63-
return opts.serverUrl.includes(':') ? opts.serverUrl : `${opts.serverUrl}:${port}`;
64-
}
65-
return serverUrl || `http://localhost:${port}`;
66-
};
67-
const runtimeServerUrl = getServerUrl();
86+
const runtimeServerUrl = resolveServerUrl(opts.serverUrl, serverUrl, port);
6887
log(`Will try to serve app at ${runtimeServerUrl}`);
6988
if (opts.manualStart) {
7089
// for debugging, you can start the dev server manually by cd'ing into the temp dir
@@ -147,13 +166,7 @@ export const application = (
147166
serve: async (opts: { port?: number; manualStart?: boolean; detached?: boolean; serverUrl?: string } = {}) => {
148167
const log = logger.child({ prefix: 'serve' }).info;
149168
const port = opts.port || (await getPort());
150-
const getServerUrl = () => {
151-
if (opts.serverUrl) {
152-
return opts.serverUrl.includes(':') ? opts.serverUrl : `${opts.serverUrl}:${port}`;
153-
}
154-
return serverUrl || `http://localhost:${port}`;
155-
};
156-
const runtimeServerUrl = getServerUrl();
169+
const runtimeServerUrl = resolveServerUrl(opts.serverUrl, serverUrl, port);
157170
log(`Will try to serve app at ${runtimeServerUrl}`);
158171

159172
if (opts.manualStart) {

0 commit comments

Comments
 (0)