Skip to content

Commit e175bb6

Browse files
authored
Add --port to ng serve commands (#9113)
1 parent 9618a7a commit e175bb6

File tree

4 files changed

+86
-68
lines changed

4 files changed

+86
-68
lines changed

src/emulator/apphosting/developmentServer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ export async function detectPackageManager(rootdir: string): Promise<PackageMana
3232
throw new FirebaseError("Unsupported package manager");
3333
}
3434

35-
export async function detectStartCommand(rootDir: string) {
35+
export async function detectPackageManagerStartCommand(rootDir: string) {
3636
try {
3737
const packageManager = await detectPackageManager(rootDir);
38+
// TODO: This is not ideal for angular apps which don't usually have a dev script.
3839
return `${packageManager} run dev`;
3940
} catch (e) {
4041
throw new FirebaseError(

src/emulator/apphosting/serve.spec.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe("serve", () => {
1717
let checkListenableStub: sinon.SinonStub;
1818
let wrapSpawnStub: sinon.SinonStub;
1919
let spawnWithCommandStringStub: sinon.SinonStub;
20-
let detectStartCommandStub: sinon.SinonStub;
20+
let detectPackageManagerStartCommandStub: sinon.SinonStub;
2121
let configsStub: sinon.SinonStubbedInstance<typeof configsImport>;
2222
let resolveProjectPathStub: sinon.SinonStub;
2323
let listRunningWithInfoStub: sinon.SinonStub;
@@ -28,22 +28,22 @@ describe("serve", () => {
2828
checkListenableStub = sinon.stub(portUtils, "checkListenable");
2929
wrapSpawnStub = sinon.stub(spawn, "wrapSpawn");
3030
spawnWithCommandStringStub = sinon.stub(spawn, "spawnWithCommandString");
31-
detectStartCommandStub = sinon.stub(utils, "detectStartCommand");
31+
detectPackageManagerStartCommandStub = sinon.stub(utils, "detectPackageManagerStartCommand");
3232
configsStub = sinon.stub(configsImport);
3333
resolveProjectPathStub = sinon.stub(projectPathImport, "resolveProjectPath");
3434

3535
listRunningWithInfoStub = sinon.stub(emulatorRegistry.EmulatorRegistry, "listRunningWithInfo");
3636
setEnvVarsForEmulatorsStub = sinon.stub(emulatorEnvs, "setEnvVarsForEmulators");
3737

3838
resolveProjectPathStub.returns("");
39-
detectStartCommandStub.returns("npm run dev");
39+
detectPackageManagerStartCommandStub.returns("npm run dev");
4040

4141
accessSecretVersionStub = sinon.stub(secrets, "accessSecretVersion");
4242
});
4343

4444
afterEach(() => {
4545
wrapSpawnStub.restore();
46-
detectStartCommandStub.restore();
46+
detectPackageManagerStartCommandStub.restore();
4747
checkListenableStub.restore();
4848
sinon.verifyAndRestore();
4949
});
@@ -83,6 +83,30 @@ describe("serve", () => {
8383
expect(spawnWithCommandStringStub.getCall(0).args[0]).to.eq(startCommand);
8484
});
8585

86+
it("should append --port if an ng serve command is detected", async () => {
87+
const startCommand = "ng serve --verbose";
88+
checkListenableStub.onFirstCall().returns(true);
89+
configsStub.getLocalAppHostingConfiguration.resolves(AppHostingYamlConfig.empty());
90+
91+
await serve.start({ startCommand });
92+
93+
expect(spawnWithCommandStringStub).to.be.called;
94+
expect(spawnWithCommandStringStub.getCall(0).args[0]).to.eq(startCommand + " --port 5002");
95+
});
96+
97+
it("should reject the custom command if a port is specified", async () => {
98+
const startCommand = "ng serve --port 5004";
99+
checkListenableStub.onFirstCall().returns(true);
100+
configsStub.getLocalAppHostingConfiguration.resolves(AppHostingYamlConfig.empty());
101+
102+
await expect(serve.start({ startCommand })).to.be.rejectedWith(
103+
FirebaseError,
104+
/Specifying a port in the start command is not supported by the apphosting emulator/,
105+
);
106+
107+
expect(spawnWithCommandStringStub).to.not.be.called;
108+
});
109+
86110
it("Should pass plaintext environment variables", async () => {
87111
const yaml = AppHostingYamlConfig.empty();
88112
yaml.env["FOO"] = { value: "BAR" };

src/emulator/apphosting/serve.ts

Lines changed: 54 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { isIPv4 } from "net";
77
import * as clc from "colorette";
88
import { checkListenable } from "../portUtils";
9-
import { detectPackageManager, detectStartCommand } from "./developmentServer";
9+
import { detectPackageManager, detectPackageManagerStartCommand } from "./developmentServer";
1010
import { DEFAULT_HOST, DEFAULT_PORTS } from "../constants";
1111
import { spawnWithCommandString } from "../../init/spawn";
1212
import { logger } from "./developmentServer";
@@ -33,32 +33,6 @@ interface StartOptions {
3333
rootDirectory?: string;
3434
}
3535

36-
/**
37-
* Spins up a project locally by running the project's dev command.
38-
*
39-
* Assumptions:
40-
* - Dev server runs on "localhost" when the package manager's dev command is
41-
* run
42-
* - Dev server will respect the PORT environment variable
43-
*/
44-
export async function start(options?: StartOptions): Promise<{ hostname: string; port: number }> {
45-
const hostname = DEFAULT_HOST;
46-
let port = options?.port ?? DEFAULT_PORTS.apphosting;
47-
while (!(await availablePort(hostname, port))) {
48-
port += 1;
49-
}
50-
51-
await serve(
52-
options?.projectId,
53-
options?.backendId,
54-
port,
55-
options?.startCommand,
56-
options?.rootDirectory,
57-
);
58-
59-
return { hostname, port };
60-
}
61-
6236
// Matches a fully qualified secret or version name, e.g.
6337
// projects/my-project/secrets/my-secret/versions/1
6438
// projects/my-project/secrets/my-secret/versions/latest
@@ -111,22 +85,58 @@ async function loadSecret(project: string | undefined, name: string): Promise<st
11185
}
11286

11387
/**
114-
* Runs the development server in a child process.
88+
* Spins up a project locally by running the project's dev command.
89+
*
90+
* Assumptions:
91+
* - Dev server runs on "localhost" when the package manager's dev command is
92+
* run
93+
* - Dev server will respect the PORT environment variable
94+
* - This is not the case for Angular. When an `ng serve`
95+
* custom command is detected, we add --port <PORT> instead.
11596
*/
116-
async function serve(
117-
projectId: string | undefined,
118-
backendId: string | undefined,
119-
port: number,
120-
startCommand?: string,
121-
backendRelativeDir?: string,
122-
): Promise<void> {
123-
backendRelativeDir = backendRelativeDir ?? "./";
97+
export async function start(options?: StartOptions): Promise<{ hostname: string; port: number }> {
98+
const hostname = DEFAULT_HOST;
99+
let port = options?.port ?? DEFAULT_PORTS.apphosting;
100+
while (!(await availablePort(hostname, port))) {
101+
port += 1;
102+
}
103+
104+
const backendRoot = resolveProjectPath({}, options?.rootDirectory ?? "./");
105+
106+
let startCommand;
107+
if (options?.startCommand) {
108+
startCommand = options?.startCommand;
109+
// Angular and nextjs CLIs allow for specifying port options but the emulator is setting and
110+
// specifying specific ports rather than use framework defaults or w/e the user has set, so we
111+
// need to reject such custom commands.
112+
// NOTE: this is not robust, a command could be a wrapper around another command and we cannot
113+
// detect --port there.
114+
if (startCommand.includes("--port") || startCommand.includes(" -p ")) {
115+
throw new FirebaseError(
116+
"Specifying a port in the start command is not supported by the apphosting emulator",
117+
);
118+
}
119+
// Angular does not respect the NodeJS.ProcessEnv.PORT set below. Port needs to be
120+
// set directly in the CLI.
121+
if (startCommand.includes("ng serve")) {
122+
startCommand += ` --port ${port}`;
123+
}
124+
logger.logLabeled(
125+
"BULLET",
126+
Emulators.APPHOSTING,
127+
`running custom start command: '${startCommand}'`,
128+
);
129+
} else {
130+
// TODO: port may be specified in an underlying command. But we will need to parse the package.json
131+
// file to be sure.
132+
startCommand = await detectPackageManagerStartCommand(backendRoot);
133+
logger.logLabeled("BULLET", Emulators.APPHOSTING, `starting app with: '${startCommand}'`);
134+
}
124135

125-
const backendRoot = resolveProjectPath({}, backendRelativeDir);
126136
const apphostingLocalConfig = await getLocalAppHostingConfiguration(backendRoot);
127137
const resolveEnv = Object.entries(apphostingLocalConfig.env).map(async ([key, value]) => [
128138
key,
129-
value.value ? value.value : await loadSecret(projectId, value.secret!),
139+
value.value ? value.value : await loadSecret(options?.projectId, value.secret!),
130140
]);
131141

132142
const environmentVariablesToInject: NodeJS.ProcessEnv = {
@@ -135,8 +145,8 @@ async function serve(
135145
...Object.fromEntries(await Promise.all(resolveEnv)),
136146
FIREBASE_APP_HOSTING: "1",
137147
X_GOOGLE_TARGET_PLATFORM: "fah",
138-
GCLOUD_PROJECT: projectId,
139-
PROJECT_ID: projectId,
148+
GCLOUD_PROJECT: options?.projectId,
149+
PROJECT_ID: options?.projectId,
140150
PORT: port.toString(),
141151
};
142152

@@ -145,7 +155,7 @@ async function serve(
145155
// TODO(jamesdaniels) look into pnpm support for autoinit
146156
logLabeledWarning("apphosting", `Firebase JS SDK autoinit does not currently support PNPM.`);
147157
} else {
148-
const webappConfig = await getBackendAppConfig(projectId, backendId);
158+
const webappConfig = await getBackendAppConfig(options?.projectId, options?.backendId);
149159
if (webappConfig) {
150160
environmentVariablesToInject["FIREBASE_WEBAPP_CONFIG"] ||= JSON.stringify(webappConfig);
151161
environmentVariablesToInject["FIREBASE_CONFIG"] ||= JSON.stringify({
@@ -157,31 +167,14 @@ async function serve(
157167
await tripFirebasePostinstall(backendRoot, environmentVariablesToInject);
158168
}
159169

160-
if (startCommand) {
161-
logger.logLabeled(
162-
"BULLET",
163-
Emulators.APPHOSTING,
164-
`running custom start command: '${startCommand}'`,
165-
);
166-
167-
// NOTE: Development server should not block main emulator process.
168-
spawnWithCommandString(startCommand, backendRoot, environmentVariablesToInject)
169-
.catch((err) => {
170-
logger.logLabeled("ERROR", Emulators.APPHOSTING, `failed to start Dev Server: ${err}`);
171-
})
172-
.then(() => logger.logLabeled("BULLET", Emulators.APPHOSTING, `Dev Server stopped`));
173-
return;
174-
}
175-
176-
const detectedStartCommand = await detectStartCommand(backendRoot);
177-
logger.logLabeled("BULLET", Emulators.APPHOSTING, `starting app with: '${detectedStartCommand}'`);
178-
179170
// NOTE: Development server should not block main emulator process.
180-
spawnWithCommandString(detectedStartCommand, backendRoot, environmentVariablesToInject)
171+
spawnWithCommandString(startCommand, backendRoot, environmentVariablesToInject)
181172
.catch((err) => {
182173
logger.logLabeled("ERROR", Emulators.APPHOSTING, `failed to start Dev Server: ${err}`);
183174
})
184175
.then(() => logger.logLabeled("BULLET", Emulators.APPHOSTING, `Dev Server stopped`));
176+
177+
return { hostname, port };
185178
}
186179

187180
function availablePort(host: string, port: number): Promise<boolean> {

src/emulator/initEmulators.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import * as clc from "colorette";
44
import { join } from "path";
55
import { input, confirm } from "../prompt";
6-
import { detectStartCommand } from "./apphosting/developmentServer";
6+
import { detectPackageManagerStartCommand } from "./apphosting/developmentServer";
77
import { EmulatorLogger } from "./emulatorLogger";
88
import { Emulators } from "./types";
99
import { Env, maybeGenerateEmulatorYaml } from "../apphosting/config";
@@ -29,7 +29,7 @@ export const AdditionalInitFns: AdditionalInitFnsType = {
2929

3030
const backendRoot = join(cwd, backendRelativeDir);
3131
try {
32-
const startCommand = await detectStartCommand(backendRoot);
32+
const startCommand = await detectPackageManagerStartCommand(backendRoot);
3333
additionalConfigs.set("startCommand", startCommand);
3434
} catch (e) {
3535
logger.log(

0 commit comments

Comments
 (0)