feat: make positional arguments explicit

BREAKING CHANGES: --api-location options cannot take an API server URL anymore, --api-server-url must now be used for this use case.
Azure · May 2, 2022 · c01e512 · c01e512
1 parent 205481d
commit c01e512
Show file tree

Hide file tree

Showing 11 changed files with 177 additions and 86 deletions.
diff --git a/schema/swa-cli.config.schema.json b/schema/swa-cli.config.schema.json
@@ -143,6 +143,14 @@
           "description": "The folder containing the source code of the API application",
           "type": "string"
         },
+        "devServerUrl": {
+          "description": "Connect to the dev server at this URL instead of using output location",
+          "type": "string"
+        },
+        "apiServerUrl": {
+          "description": "Connect to the api server at this URL instead of using api location",
+          "type": "string"
+        },
         "apiPort": {
           "description": "The API server port passed to func start",
           "type": "number"

diff --git a/src/cli/commands/build.ts b/src/cli/commands/build.ts
@@ -5,22 +5,31 @@ import { execSync } from 'child_process';
 import { DEFAULT_CONFIG } from "../../config";
 import { detectProjectFolders, generateConfiguration } from "../../core/frameworks";
 import {
-  configureOptions, findUpPackageJsonDir, isUserOrConfigOption, logger, pathExists, readWorkflowFile, swaCliConfigFilename,
+  configureOptions, findUpPackageJsonDir, isUserOption, isUserOrConfigOption, logger, matchLoadedConfigName, pathExists, readWorkflowFile, swaCliConfigFilename,
 } from "../../core/utils";
 
 export default function registerCommand(program: Command) {
   program
-    .command("build [configurationName|outputLocation]")
-    .usage("[configurationName|outputLocation] [options]")
+    .command("build [configName|outputLocation]")
+    .usage("[configName|outputLocation] [options]")
     .description("build your project")
     .option("--app-location <appLocation>", "the folder containing the source code of the front-end application", DEFAULT_CONFIG.appLocation)
     .option("--api-location <apiLocation>", "the folder containing the source code of the API application", DEFAULT_CONFIG.apiLocation)
     .option("--output-location <outputLocation>", "the folder where the front-end public files are location", DEFAULT_CONFIG.outputLocation)
     .option("--app-build-command <command>", "the command used to build your app", DEFAULT_CONFIG.appBuildCommand)
     .option("--api-build-command <command>", "the command used to build your api", DEFAULT_CONFIG.apiBuildCommand)
     .option("--auto", "automatically detect how to build your app and api", false)
-    .action(async (configOrOutputLocation: string = `.${path.sep}`, _options: SWACLIConfig, command: Command) => {
-      const options = await configureOptions(configOrOutputLocation, command.optsWithGlobals(), command, "build");
+    .action(async (positionalArg: string = `.${path.sep}`, _options: SWACLIConfig, command: Command) => {
+      const options = await configureOptions(positionalArg, command.optsWithGlobals(), command, "build");
+      if (!matchLoadedConfigName(positionalArg)) {
+        if (isUserOption('outputLocation')) {
+          logger.error(`outputLocation was set on both positional argument and option.`, true);
+        }
+
+        // If it's not the config name, then it's the output location
+        options.outputLocation = positionalArg;
+      }
+
       await build(options);
     });
 }

diff --git a/src/cli/commands/deploy.spec.ts b/src/cli/commands/deploy.spec.ts
@@ -67,12 +67,13 @@ describe("deploy", () => {
   });
 
   it("should return a Promise", () => {
-    expect(deploy("./dist", {})).toBeInstanceOf(Promise);
+    expect(deploy({ outputLocation: "./dist" })).toBeInstanceOf(Promise);
   });
 
   it("should print warning when using dry run mode", async () => {
     mockFs();
-    await deploy("./dist", {
+    await deploy({
+      outputLocation: "./dist",
       dryRun: true,
     });
     expect(logger.warn).toHaveBeenNthCalledWith(1, "***********************************************************************");
@@ -82,7 +83,8 @@ describe("deploy", () => {
 
   it.skip("should print error and exit when --api-location does not exist", async () => {
     mockFs();
-    await deploy("./dist", {
+    await deploy({
+      outputLocation: "./dist",
       apiLocation: "/does/not/exist",
     });
     expect(logger.error).toHaveBeenNthCalledWith(1, `The provided API folder /does/not/exist does not exist. Abort.`, true);
@@ -91,7 +93,8 @@ describe("deploy", () => {
   it.skip("should print an error and exit, if --deployment-token is not provided and login failed", async () => {
     jest.spyOn(loginModule, "login").mockImplementation(() => Promise.reject("mock-error"));
 
-    await deploy("./dist", {
+    await deploy({
+      outputLocation: "./dist",
       apiLocation: "./api",
       dryRun: false,
     });
@@ -112,7 +115,8 @@ describe("deploy", () => {
   });
 
   it.skip("should accept a deploymentToken provided via --deployment-token", async () => {
-    await deploy("./dist", {
+    await deploy({
+      outputLocation: "./dist",
       apiLocation: "./api",
       deploymentToken: "123",
       dryRun: false,
@@ -148,7 +152,8 @@ describe("deploy", () => {
   it.skip("should accept a deploymentToken provided via the environment variable SWA_CLI_DEPLOYMENT_TOKEN", async () => {
     process.env.SWA_CLI_DEPLOYMENT_TOKEN = "123";
 
-    await deploy("./dist", {
+    await deploy({
+      outputLocation: "./dist",
       apiLocation: "./api",
       dryRun: false,
     });

diff --git a/src/cli/commands/deploy.ts b/src/cli/commands/deploy.ts
@@ -9,8 +9,10 @@ import {
   configureOptions,
   findSWAConfigFile,
   getCurrentSwaCliConfigFromFile,
+  isUserOption,
   logger,
   logGiHubIssueMessageAndExit,
+  matchLoadedConfigName,
   readWorkflowFile,
   updateSwaCliConfigFile,
 } from "../../core";
@@ -23,18 +25,27 @@ const packageInfo = require(path.join(__dirname, "..", "..", "..", "package.json
 
 export default function registerCommand(program: Command) {
   const deployCommand = program
-    .command("deploy [outputLocation]")
-    .usage("[outputLocation] [options]")
+    .command("deploy [configName|outputLocation]")
+    .usage("[configName|outputLocation] [options]")
     .description("Deploy the current project to Azure Static Web Apps")
     .option("--app-location <appLocation>", "the folder containing the source code of the front-end application", DEFAULT_CONFIG.appLocation)
     .option("--api-location <apiLocation>", "the folder containing the source code of the API application", DEFAULT_CONFIG.apiLocation)
     .option("--deployment-token <secret>", "the secret token used to authenticate with the Static Web Apps")
     .option("--dry-run", "simulate a deploy process without actually running it", DEFAULT_CONFIG.dryRun)
     .option("--print-token", "print the deployment token", false)
     .option("--env [environment]", "the type of deployment environment where to deploy the project", DEFAULT_CONFIG.env)
-    .action(async (outputLocation: string = `.${path.sep}`, _options: SWACLIConfig, command: Command) => {
-      const options = await configureOptions(outputLocation, command.optsWithGlobals(), command, "deploy");
-      await deploy(options.outputLocation ?? outputLocation, options);
+    .action(async (positionalArg: string = `.${path.sep}`, _options: SWACLIConfig, command: Command) => {
+      const options = await configureOptions(positionalArg, command.optsWithGlobals(), command, "deploy");
+      if (!matchLoadedConfigName(positionalArg)) {
+        if (isUserOption('outputLocation')) {
+          logger.error(`outputLocation was set on both positional argument and option.`, true);
+        }
+
+        // If it's not the config name, then it's the output location
+        options.outputLocation = positionalArg;
+      }
+
+      await deploy(options);
     })
     .addHelpText(
       "after",
@@ -61,12 +72,11 @@ Examples:
   addSharedLoginOptionsToCommand(deployCommand);
 }
 
-export async function deploy(outputLocationOrConfigName: string, options: SWACLIConfig) {
+export async function deploy(options: SWACLIConfig) {
   const { SWA_CLI_DEPLOYMENT_TOKEN, SWA_CLI_DEBUG } = swaCLIEnv();
   const isVerboseEnabled = SWA_CLI_DEBUG === "silly";
 
-  let { appLocation, apiLocation, dryRun, deploymentToken, printToken, appName, swaConfigLocation, verbose } = options;
-  let outputLocation = outputLocationOrConfigName;
+  let { appLocation, apiLocation, outputLocation, dryRun, deploymentToken, printToken, appName, swaConfigLocation, verbose } = options;
 
   if (dryRun) {
     logger.warn("***********************************************************************");

diff --git a/src/cli/commands/init.spec.ts b/src/cli/commands/init.spec.ts
@@ -33,7 +33,7 @@ describe("swa init", () => {
   it("should create a config file", async () => {
     mockFs();
 
-    await init("test", { ...defaultCliConfig, yes: true });
+    await init({ ...defaultCliConfig, configName: "test", yes: true });
     const configFile = fs.readFileSync(defaultCliConfig.config, "utf-8");
 
     expect(configFile).toMatchInlineSnapshot(`
@@ -42,13 +42,7 @@ describe("swa init", () => {
         \\"configurations\\": {
           \\"test\\": {
             \\"appLocation\\": \\"./\\",
-            \\"outputLocation\\": \\"./\\",
-            \\"start\\": {
-              \\"outputLocation\\": \\"./\\"
-            },
-            \\"deploy\\": {
-              \\"outputLocation\\": \\"./\\"
-            }
+            \\"outputLocation\\": \\"./\\"
           }
         }
       }"
@@ -59,7 +53,7 @@ describe("swa init", () => {
     mockFs();
     const promptsMock = jest.requireMock("prompts");
 
-    await init(undefined, { ...defaultCliConfig, yes: true });
+    await init({ ...defaultCliConfig, yes: true });
     expect(promptsMock).not.toHaveBeenCalled();
   });
 
@@ -68,7 +62,7 @@ describe("swa init", () => {
     const promptsMock = jest.requireMock("prompts");
     promptsMock.mockResolvedValue(defautResolvedPrompts);
 
-    await init(undefined, { ...defaultCliConfig });
+    await init({ ...defaultCliConfig });
 
     // check that the first prompt ask for configName property
     expect(promptsMock.mock.calls[0][0].name).toEqual("configName");
@@ -79,7 +73,7 @@ describe("swa init", () => {
     const promptsMock = jest.requireMock("prompts");
     promptsMock.mockResolvedValue(defautResolvedPrompts);
 
-    await init("my-app", { ...defaultCliConfig });
+    await init({ ...defaultCliConfig, configName: "my-app" });
     const configJson = JSON.parse(fs.readFileSync(defaultCliConfig.config, "utf-8"));
 
     // check that the first prompt ask for configName property
@@ -92,8 +86,8 @@ describe("swa init", () => {
     const promptsMock: jest.Mock = jest.requireMock("prompts");
     promptsMock.mockResolvedValue({ ...defautResolvedPrompts, confirmOverwrite: false });
 
-    await init("test", { ...defaultCliConfig, yes: true });
-    await init("test", { ...defaultCliConfig });
+    await init({ ...defaultCliConfig, configName: "test", yes: true });
+    await init({ ...defaultCliConfig, configName: "test" });
 
     const configJson = JSON.parse(fs.readFileSync(defaultCliConfig.config, "utf-8"));
     const lastCall = promptsMock.mock.calls.length - 1;
@@ -106,8 +100,8 @@ describe("swa init", () => {
     const promptsMock: jest.Mock = jest.requireMock("prompts");
     promptsMock.mockResolvedValue(defautResolvedPrompts);
 
-    await init("test", { ...defaultCliConfig, yes: true });
-    await init("test", { ...defaultCliConfig });
+    await init({ ...defaultCliConfig, configName: "test", yes: true });
+    await init({ ...defaultCliConfig, configName: "test" });
 
     const configJson = JSON.parse(fs.readFileSync(defaultCliConfig.config, "utf-8"));
     const lastCall = promptsMock.mock.calls.length - 1;
@@ -118,7 +112,7 @@ describe("swa init", () => {
   it("should detect frameworks and create a config file", async () => {
     mockFs({ src: mockFs.load("e2e/fixtures/static-node-ts") });
 
-    await init("test", { ...defaultCliConfig, yes: true });
+    await init({ ...defaultCliConfig, configName: "test", yes: true });
     const configFile = fs.readFileSync(defaultCliConfig.config, "utf-8");
 
     expect(configFile).toMatchInlineSnapshot(`
@@ -128,14 +122,32 @@ describe("swa init", () => {
           \\"test\\": {
             \\"appLocation\\": \\"src/\\",
             \\"apiLocation\\": \\"node-ts/dist\\",
-            \\"outputLocation\\": \\"src/\\",
+            \\"outputLocation\\": \\".\\",
+            \\"apiBuildCommand\\": \\"npm run build --if-present\\"
+          }
+        }
+      }"
+    `);
+  });
+
+  it("should detect frameworks and create a config file including a dev server", async () => {
+    mockFs({ src: mockFs.load("e2e/fixtures/astro-node") });
+
+    await init({ ...defaultCliConfig, configName: "test", yes: true });
+    const configFile = fs.readFileSync(defaultCliConfig.config, "utf-8");
+
+    expect(configFile).toMatchInlineSnapshot(`
+      "{
+        \\"$schema\\": \\"https://aka.ms/azure/static-web-apps-cli/schema\\",
+        \\"configurations\\": {
+          \\"test\\": {
+            \\"appLocation\\": \\".\\",
+            \\"apiLocation\\": \\"src/node\\",
+            \\"outputLocation\\": \\"src/astro-preact/_site\\",
+            \\"appBuildCommand\\": \\"npm run build\\",
             \\"apiBuildCommand\\": \\"npm run build --if-present\\",
-            \\"start\\": {
-              \\"outputLocation\\": \\"src/\\"
-            },
-            \\"deploy\\": {
-              \\"outputLocation\\": \\"src/\\"
-            }
+            \\"run\\": \\"npm run dev\\",
+            \\"devServerUrl\\": \\"http://localhost:8080\\"
           }
         }
       }"

diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts
@@ -7,6 +7,7 @@ import {
   configureOptions,
   dasherize,
   hasConfigurationNameInConfigFile,
+  isUserOption,
   logger,
   swaCliConfigFileExists,
   swaCliConfigFilename,
@@ -16,21 +17,29 @@ import { detectProjectFolders, generateConfiguration, isDescendantPath } from ".
 
 export default function registerCommand(program: Command) {
   program
-    .command("init [configurationName]")
-    .usage("[configurationName] [options]")
+    .command("init [configName]")
+    .usage("[configName] [options]")
     .description("initialize a new static web app project")
     .option("--yes", "answer yes to all prompts (disable interactive mode)", false)
-    .action(async (configurationName: string, _options: SWACLIConfig, command: Command) => {
+    .action(async (configName: string | undefined, _options: SWACLIConfig, command: Command) => {
       const options = await configureOptions(undefined, command.optsWithGlobals(), command, "init");
-      await init(configurationName, options);
+      if (configName) {
+        if (isUserOption('configName')) {
+          logger.error(`configName was set on both positional argument and option.`, true);
+        }
+
+        options.configName = configName;
+      }
+
+      await init(options);
     });
 }
 
-export async function init(name: string | undefined, options: SWACLIConfig, showHints: boolean = true) {
+export async function init(options: SWACLIConfig, showHints: boolean = true) {
   const configFilePath = options.config!;
   const disablePrompts = options.yes ?? false;
   const outputFolder = process.cwd();
-  let configName: string = name?.trim() ?? "";
+  let configName: string = options.configName?.trim() ?? "";
 
   if (configName === "") {
     const response = await promptOrUseDefault(disablePrompts, {
@@ -149,12 +158,7 @@ function convertToCliConfig(config: FrameworkConfig): SWACLIConfig {
     appBuildCommand: config.appBuildCommand,
     apiBuildCommand: config.apiBuildCommand,
     run: config.devServerCommand,
-    start: {
-      outputLocation: config.devServerUrl || config.outputLocation,
-    },
-    deploy: {
-      outputLocation: config.outputLocation,
-    },
+    devServerUrl: config.devServerUrl,
   };
 }