Repository: firebase/firebase-tools (ts)

Issue:
Error: Hosting target (project-name) not detected in firebase.json
<!-- DO NOT DELETE
validate_template=true
template_path=.github/ISSUE_TEMPLATE/bug_report.md
-->

<!--
Thank you for contributing to the Firebase community!

Think you found a bug?
=======================
Yeah, we're definitely not perfect! Please use this template and include a minimal repro when opening the issue. If you know how to solve the issue, please create a Pull Request, and we'd be happy to review it!

Have a feature request?
========================
Great, we love hearing how we can improve our products! However, do not use this template to submit a feature request. Please submit your feature requests to: https://firebase.google.com/support/contact/bugs-features/

Have a usage question?
=======================
We get lots of those and we love helping you, but GitHub is not the best place for them and they will be closed. Please take a look at the guide first: https://firebase.google.com/docs/cli/

If the official documentation doesn't help, try asking through our official support channel: https://firebase.google.com/support/

Additional locations to check for solutions or assistance from the community:
- Stack Overflow: https://stackoverflow.com/
- Firebase Slack Community: https://firebase.community/

*Please avoid duplicate posting across multiple channels!*
-->

### [REQUIRED] Environment info

<!-- What version of the Firebase CLI (`firebase-tools`) are you using? Note that your issue may already be fixed in the latest versions. The latest version can be found at https://github.com/firebase/firebase-tools/releases -->

<!-- Output of `firebase --version` -->

**firebase-tools:**v8.13.1

<!-- e.g. macOS, Windows, Ubuntu -->

**Platform:**macOS

### [REQUIRED] Test case

`firebase deploy --only hosting:(project-name)`

<!-- Provide a minimal, complete, and verifiable example (http://stackoverflow.com/help/mcve) -->

### [REQUIRED] Steps to reproduce

I'm using the following Github action: https://github.com/w9jds/firebase-action/ which uses the latest release of firebase-tools.
Since the last update, it's not working anymore. I had to manually set to an older version of firebase-tools (I went for v8.12.1, I haven't tried with v8.13.0)

<!-- Provide the steps needed to reproduce the issue with the above test case. -->

### [REQUIRED] Expected behavior

Successful deploy
<!-- What is the expected behavior? -->

### [REQUIRED] Actual behavior
Deploy error
Error: Hosting target (project-name) not detected in firebase.json

<!-- Run the command with --debug flag, and include the logs below. -->


Pull request:
allow --only hosting:site with bare hosting configs without targets
<!--

Thank you for contributing to the Firebase community! Please fill out the form below.

Run the linter and test suite
==============================
Run `npm test` to make sure your changes compile properly and the tests all pass on your local machine. We've hooked up this repo with continuous integration to double check those things for you.

-->

### Description

Fixes #2731 

In `v8.12.1`, we did not do any validation of `target` in `--only hosting:<target>`, but _did_ allow the ability to specify `--only hosting:<site-name>` because of how the filtering logic played out when a user had a single Hosting config (think immediately post `init hosting`).

In #2704, we started validating that `target` was a valid Hosting target, as our documentation states that `target` must match a `target` in `firebase.json`. This is well meaning, but it broke the case where the Hosting site could be specified by name.

Specifying a Hosting `target` is intended for [multi-site setups, described in the documentation](https://firebase.google.com/docs/hosting/multisites#set_up_deploy_targets). However, we also want to maintain support for specifying the `site` in a multisite configuration (rather than `target`).

### Scenarios Tested

`firebase deploy` with
- nothing
  - on a basic config
    - with a `target` specified in firebase.json
  - on a multi-site config
   - with `site` instead of `target`
- `--only hosting`
  - on a basic config
    - with a `target` specified in firebase.json
  - on a multi-site config
   - with `site` instead of `target`
- `--only hosting:<sitename>`
  - on a basic config (works)
    - with a `target` specified in firebase.json (fails)
  - on a multi-site config (fails)
   - with `site` instead of `target` (works, since it's the site)
- `only hosting:<target>`
  - on a basic config (fails, no target)
    - with a `target` specified in firebase.json (works)
  - on a multi-site config (works)
   - with `site` instead of `target` (fails, since it's not a target)

I've also added new tests for `normalizedHostingConfigs` to help solidify this behavior (yay 100% test coverage).

cc @thomasmburke 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e69de29bb..c8cbf5f7c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -0,0 +1,1 @@
+- Restores the ability to specify a Hosting `site` (configured in `firebase.json`) as a deploy `--only` target. (#2731)
diff --git a/src/hosting/normalizedHostingConfigs.ts b/src/hosting/normalizedHostingConfigs.ts
index 11bc575c0..2744d1707 100644
--- a/src/hosting/normalizedHostingConfigs.ts
+++ b/src/hosting/normalizedHostingConfigs.ts
@@ -24,16 +24,35 @@ function filterOnly(configs: HostingConfig[], onlyString: string): HostingConfig
     .filter((target) => target.startsWith("hosting:"))
     .map((target) => target.replace("hosting:", ""));
 
-  // Check to see that all the hosting deploy targets exist in the hosting config
-  onlyTargets.forEach((onlyTarget) => {
-    if (!configs.some((config) => config.target === onlyTarget)) {
-      throw new FirebaseError(`Hosting target ${bold(onlyTarget)} not detected in firebase.json`);
+  const configsBySite = new Map<string, HostingConfig>();
+  const configsByTarget = new Map<string, HostingConfig>();
+  for (const c of configs) {
+    if (c.site) {
+      configsBySite.set(c.site, c);
     }
-  });
+    if (c.target) {
+      configsByTarget.set(c.target, c);
+    }
+  }
+
+  const filteredConfigs: HostingConfig[] = [];
+  // Check to see that all the hosting deploy targets exist in the hosting
+  // config as either `site`s or `target`s.
+  for (const onlyTarget of onlyTargets) {
+    if (configsBySite.has(onlyTarget)) {
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      filteredConfigs.push(configsBySite.get(onlyTarget)!);
+    } else if (configsByTarget.has(onlyTarget)) {
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      filteredConfigs.push(configsByTarget.get(onlyTarget)!);
+    } else {
+      throw new FirebaseError(
+        `Hosting site or target ${bold(onlyTarget)} not detected in firebase.json`
+      );
+    }
+  }
 
-  return configs.filter((config: HostingConfig) =>
-    onlyTargets.includes(config.target || config.site)
-  );
+  return filteredConfigs;
 }
 
 /**
@@ -61,10 +80,18 @@ export function normalizedHostingConfigs(
     configs = [configs];
   }
 
-  configs = filterOnly(configs, cmdOptions.only);
+  for (const c of configs) {
+    if (c.target && c.site) {
+      throw new FirebaseError(
+        `Hosting configs should only include either "site" or "target", not both.`
+      );
+    }
+  }
+
+  const hostingConfigs: HostingConfig[] = filterOnly(configs, cmdOptions.only);
 
   if (options.resolveTargets) {
-    configs.forEach((cfg: HostingConfig) => {
+    for (const cfg of hostingConfigs) {
       if (cfg.target) {
         const matchingTargets = cmdOptions.rc.requireTarget(
           cmdOptions.project,
@@ -82,8 +109,8 @@ export function normalizedHostingConfigs(
       } else if (!cfg.site) {
         throw new FirebaseError('Must supply either "site" or "target" in each "hosting" config.');
       }
-    });
+    }
   }
 
-  return configs;
+  return hostingConfigs;
 }


Test patch:
diff --git a/src/test/hosting/normalizedHostingConfigs.spec.ts b/src/test/hosting/normalizedHostingConfigs.spec.ts
new file mode 100644
index 000000000..6fa9a80ed
--- /dev/null
+++ b/src/test/hosting/normalizedHostingConfigs.spec.ts
@@ -0,0 +1,176 @@
+import { expect } from "chai";
+import { FirebaseError } from "../../error";
+
+import { normalizedHostingConfigs } from "../../hosting/normalizedHostingConfigs";
+
+describe("normalizedHostingConfigs", () => {
+  it("should fail if both site and target are specified", () => {
+    const singleHostingConfig = { site: "site", target: "target" };
+    const cmdConfig = {
+      site: "default-site",
+      config: { get: () => singleHostingConfig },
+    };
+    expect(() => normalizedHostingConfigs(cmdConfig)).to.throw(
+      FirebaseError,
+      /configs should only include either/
+    );
+
+    const hostingConfig = [{ site: "site", target: "target" }];
+    const newCmdConfig = {
+      site: "default-site",
+      config: { get: () => hostingConfig },
+    };
+    expect(() => normalizedHostingConfigs(newCmdConfig)).to.throw(
+      FirebaseError,
+      /configs should only include either/
+    );
+  });
+
+  describe("without an only parameter", () => {
+    const DEFAULT_SITE = "default-hosting-site";
+    const baseConfig = { public: "public", ignore: ["firebase.json"] };
+    const tests = [
+      {
+        desc: "a normal hosting config",
+        cfg: Object.assign({}, baseConfig),
+        want: [Object.assign({}, baseConfig, { site: DEFAULT_SITE })],
+      },
+      {
+        desc: "no hosting config",
+        want: [],
+      },
+      {
+        desc: "a normal hosting config with a target",
+        cfg: Object.assign({}, baseConfig, { target: "main" }),
+        want: [Object.assign({}, baseConfig, { target: "main" })],
+      },
+      {
+        desc: "a hosting config with multiple targets",
+        cfg: [
+          Object.assign({}, baseConfig, { target: "t-one" }),
+          Object.assign({}, baseConfig, { target: "t-two" }),
+        ],
+        want: [
+          Object.assign({}, baseConfig, { target: "t-one" }),
+          Object.assign({}, baseConfig, { target: "t-two" }),
+        ],
+      },
+    ];
+
+    for (const t of tests) {
+      it(`should be able to parse ${t.desc}`, () => {
+        const cmdConfig = {
+          site: DEFAULT_SITE,
+          config: { get: () => t.cfg },
+        };
+        const got = normalizedHostingConfigs(cmdConfig);
+        expect(got).to.deep.equal(t.want);
+      });
+    }
+  });
+
+  describe("with an only parameter, resolving targets", () => {
+    const DEFAULT_SITE = "default-hosting-site";
+    const TARGETED_SITE = "targeted-site";
+    const baseConfig = { public: "public", ignore: ["firebase.json"] };
+    const tests = [
+      {
+        desc: "a normal hosting config, specifying the default site",
+        cfg: Object.assign({}, baseConfig),
+        only: `hosting:${DEFAULT_SITE}`,
+        want: [Object.assign({}, baseConfig, { site: DEFAULT_SITE })],
+      },
+      {
+        desc: "a hosting config with multiple sites, no targets, specifying the second site",
+        cfg: [
+          Object.assign({}, baseConfig, { site: DEFAULT_SITE }),
+          Object.assign({}, baseConfig, { site: "different-site" }),
+        ],
+        only: `hosting:different-site`,
+        want: [Object.assign({}, baseConfig, { site: "different-site" })],
+      },
+      {
+        desc: "a normal hosting config with a target",
+        cfg: Object.assign({}, baseConfig, { target: "main" }),
+        only: "hosting:main",
+        want: [Object.assign({}, baseConfig, { target: "main", site: TARGETED_SITE })],
+      },
+      {
+        desc: "a hosting config with multiple targets, specifying one",
+        cfg: [
+          Object.assign({}, baseConfig, { target: "t-one" }),
+          Object.assign({}, baseConfig, { target: "t-two" }),
+        ],
+        only: "hosting:t-two",
+        want: [Object.assign({}, baseConfig, { target: "t-two", site: TARGETED_SITE })],
+      },
+      {
+        desc: "a hosting config with multiple targets, specifying all hosting",
+        cfg: [
+          Object.assign({}, baseConfig, { target: "t-one" }),
+          Object.assign({}, baseConfig, { target: "t-two" }),
+        ],
+        only: "hosting",
+        want: [
+          Object.assign({}, baseConfig, { target: "t-one", site: TARGETED_SITE }),
+          Object.assign({}, baseConfig, { target: "t-two", site: TARGETED_SITE }),
+        ],
+      },
+      {
+        desc: "a hosting config with multiple targets, specifying an invalid target",
+        cfg: [
+          Object.assign({}, baseConfig, { target: "t-one" }),
+          Object.assign({}, baseConfig, { target: "t-two" }),
+        ],
+        only: "hosting:t-three",
+        wantErr: /Hosting site or target.+t-three.+not detected/,
+      },
+      {
+        desc: "a hosting config with multiple targets, with multiple matching targets",
+        cfg: [
+          Object.assign({}, baseConfig, { target: "t-one" }),
+          Object.assign({}, baseConfig, { target: "t-one" }),
+        ],
+        only: "hosting:t-one",
+        targetedSites: [TARGETED_SITE, TARGETED_SITE],
+        wantErr: /Hosting target.+t-one.+linked to multiple sites/,
+      },
+      {
+        desc: "a hosting config with multiple sites but no targets, only all hosting",
+        cfg: [Object.assign({}, baseConfig), Object.assign({}, baseConfig)],
+        only: "hosting",
+        wantErr: /Must supply either "site" or "target"/,
+      },
+      {
+        desc: "a hosting config with multiple sites but no targets, only an invalid target",
+        cfg: [Object.assign({}, baseConfig), Object.assign({}, baseConfig)],
+        only: "hosting:t-one",
+        wantErr: /Hosting site or target.+t-one.+not detected/,
+      },
+    ];
+
+    for (const t of tests) {
+      it(`should be able to parse ${t.desc}`, () => {
+        if (!Array.isArray(t.targetedSites)) {
+          t.targetedSites = [TARGETED_SITE];
+        }
+        const cmdConfig = {
+          site: DEFAULT_SITE,
+          only: t.only,
+          config: { get: () => t.cfg },
+          rc: { requireTarget: () => t.targetedSites },
+        };
+
+        if (t.wantErr) {
+          expect(() => normalizedHostingConfigs(cmdConfig, { resolveTargets: true })).to.throw(
+            FirebaseError,
+            t.wantErr
+          );
+        } else {
+          const got = normalizedHostingConfigs(cmdConfig, { resolveTargets: true });
+          expect(got).to.deep.equal(t.want);
+        }
+      });
+    }
+  });
+});