diff --git a/src/command.ts b/src/command.ts index 495527f4fff..f365b8fc306 100644 --- a/src/command.ts +++ b/src/command.ts @@ -165,8 +165,7 @@ export class Command { if (this.aliases) { cmd.aliases(this.aliases); } - this.options.forEach((args) => { - const flags = args.shift(); + this.options.forEach(([flags, ...args]) => { cmd.option(flags, ...args); }); diff --git a/src/emulator/apphosting/serve.spec.ts b/src/emulator/apphosting/serve.spec.ts index 7e388a9a33e..e5930c45820 100644 --- a/src/emulator/apphosting/serve.spec.ts +++ b/src/emulator/apphosting/serve.spec.ts @@ -94,17 +94,27 @@ describe("serve", () => { expect(spawnWithCommandStringStub.getCall(0).args[0]).to.eq(startCommand + " --port 5002"); }); - it("should reject the custom command if a port is specified", async () => { - const startCommand = "ng serve --port 5004"; + it("should allow custom command with port specified and show warning", async () => { + const startCommand = "npm run dev -- --port 5004"; checkListenableStub.onFirstCall().returns(true); configsStub.getLocalAppHostingConfiguration.resolves(AppHostingYamlConfig.empty()); - await expect(serve.start({ startCommand })).to.be.rejectedWith( - FirebaseError, - /Specifying a port in the start command is not supported by the apphosting emulator/, - ); + await serve.start({ startCommand }); + + expect(spawnWithCommandStringStub).to.be.called; + expect(spawnWithCommandStringStub.getCall(0).args[0]).to.eq(startCommand); + // Should log a warning about port mismatch + }); + + it("should not add port to ng serve if already specified", async () => { + const startCommand = "ng serve --port 5002"; + checkListenableStub.onFirstCall().returns(true); + configsStub.getLocalAppHostingConfiguration.resolves(AppHostingYamlConfig.empty()); + + await serve.start({ startCommand }); - expect(spawnWithCommandStringStub).to.not.be.called; + expect(spawnWithCommandStringStub).to.be.called; + expect(spawnWithCommandStringStub.getCall(0).args[0]).to.eq(startCommand); }); it("Should pass plaintext environment variables", async () => { diff --git a/src/emulator/apphosting/serve.ts b/src/emulator/apphosting/serve.ts index bc1a023b1fc..0e8b2f475ee 100644 --- a/src/emulator/apphosting/serve.ts +++ b/src/emulator/apphosting/serve.ts @@ -106,19 +106,13 @@ export async function start(options?: StartOptions): Promise<{ hostname: string; let startCommand; if (options?.startCommand) { startCommand = options?.startCommand; - // Angular and nextjs CLIs allow for specifying port options but the emulator is setting and - // specifying specific ports rather than use framework defaults or w/e the user has set, so we - // need to reject such custom commands. - // NOTE: this is not robust, a command could be a wrapper around another command and we cannot - // detect --port there. - if (startCommand.includes("--port") || startCommand.includes(" -p ")) { - throw new FirebaseError( - "Specifying a port in the start command is not supported by the apphosting emulator", - ); - } // Angular does not respect the NodeJS.ProcessEnv.PORT set below. Port needs to be - // set directly in the CLI. - if (startCommand.includes("ng serve")) { + // set directly in the CLI. Only add --port if not already specified by the user. + if ( + startCommand.includes("ng serve") && + !startCommand.includes("--port") && + !startCommand.includes(" -p ") + ) { startCommand += ` --port ${port}`; } logger.logLabeled( @@ -126,6 +120,18 @@ export async function start(options?: StartOptions): Promise<{ hostname: string; Emulators.APPHOSTING, `running custom start command: '${startCommand}'`, ); + // Warn if user specified a port that differs from the emulator port + const portMatch = startCommand.match(/--port[= ](\d+)|-p[= ](\d+)/); + if (portMatch) { + const userPort = parseInt(portMatch[1] || portMatch[2]); + if (userPort !== port) { + logLabeledWarning( + Emulators.APPHOSTING, + `Custom start command specifies port ${userPort}, but emulator is using port ${port}. ` + + `Make sure your command uses the PORT environment variable or matches the emulator port.`, + ); + } + } } else { // TODO: port may be specified in an underlying command. But we will need to parse the package.json // file to be sure.