fix: #8168 - enforce webframeworks only when needed

CHANGELOG.md

@@ -0,0 +1 @@
+- Enforce webframeworks enablement only on webframeworks sites (#8168)

src/deploy/hosting/deploy.spec.ts

@@ -0,0 +1,122 @@
+import { expect } from "chai";
+import * as sinon from "sinon";
+
+import { setEnabled } from "../../experiments";
+import { FirebaseConfig } from "../../firebaseConfig";
+import * as frameworks from "../../frameworks";
+import * as config from "../../hosting/config";
+import { HostingOptions } from "../../hosting/options";
+import { Options } from "../../options";
+import { matchesHostingTarget, prepareFrameworksIfNeeded } from "../index";
+
+describe("hosting prepare", () => {
+  let frameworksStub: sinon.SinonStubbedInstance<typeof frameworks>;
+  let classicSiteConfig: config.HostingResolved;
+  let webFrameworkSiteConfig: config.HostingResolved;
+  let firebaseJson: FirebaseConfig;
+  let options: HostingOptions & Options;
+
+  beforeEach(() => {
+    frameworksStub = sinon.stub(frameworks);
+
+    // We're intentionally using pointer references so that editing site
+    // edits the results of hostingConfig() and changes firebase.json
+    classicSiteConfig = {
+      site: "classic",
+      target: "classic",
+      public: ".",
+    };
+    webFrameworkSiteConfig = {
+      site: "webframework",
+      target: "webframework",
+      source: "src",
+    };
+    firebaseJson = {
+      hosting: [classicSiteConfig, webFrameworkSiteConfig],
+    };
+    options = {
+      cwd: ".",
+      configPath: ".",
+      only: "hosting",
+      except: "",
+      filteredTargets: ["HOSTING"],
+      force: false,
+      json: false,
+      nonInteractive: false,
+      interactive: true,
+      debug: false,
+      projectId: "project",
+      config: {
+        src: firebaseJson,
+        get: (key: string) => {
+          if (key === "hosting") {
+            return firebaseJson.hosting;
+          }
+          return null;
+        },
+      } as any,
+      rc: null as any,
+
+      // Forces caching behavior of hostingConfig call
+      normalizedHostingConfig: [classicSiteConfig, webFrameworkSiteConfig],
+    };
+  });
+
+  afterEach(() => {
+    sinon.verifyAndRestore();
+  });
+
+  describe("matchesHostingTarget", () => {
+    it("should correctly identify if a hosting target should be included in deployment", () => {
+      const cases = [
+        { only: "hosting:site1", target: "site1", expected: true },
+        { only: "hosting:site12", target: "site1", expected: false },
+        { only: "hosting:", target: "", expected: true },
+        { only: "functions:fn1", target: "site1", expected: true },
+        { only: "hosting:site1,hosting:site2", target: "site1", expected: true },
+        { only: undefined, target: "site1", expected: true },
+        { only: "hosting:site1,functions:fn1", target: "site1", expected: true },
+      ];
+
+      cases.forEach(({ only, target, expected }) => {
+        expect(matchesHostingTarget(only, target)).to.equal(expected);
+      });
+    });
+  });
+
+  it("deploys classic site without webframeworks disabled", async () => {
+    setEnabled("webframeworks", false);
+    options.only = "hosting:classic";
+    await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.not.be.rejected;
+  });
+
+  it("fails webframework deploy with webframeworks disabled", async () => {
+    setEnabled("webframeworks", false);
+    options.only = "hosting:webframework";
+    await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.be.rejectedWith(
+      /Cannot deploy a web framework from source because the experiment.+webframeworks.+is not enabled/,
+    );
+  });
+
+  it("deploys webframework site with webframeworks enabled", async () => {
+    setEnabled("webframeworks", true);
+    options.only = "hosting:webframework";
+    await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.not.be.rejected;
+    expect(frameworksStub.prepareFrameworks).to.have.been.calledOnceWith("deploy", ["hosting"]);
+  });
+
+  it("deploys classic site with webframeworks enabled", async () => {
+    setEnabled("webframeworks", true);
+    options.only = "hosting:classic";
+    await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.not.be.rejected;
+    expect(frameworksStub.prepareFrameworks).to.not.have.been.called;
+  });
+
+  it("fails when at least one site has webframeworks enabled and the experiment is disabled", async () => {
+    setEnabled("webframeworks", false);
+    options.only = "hosting";
+    await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.be.rejectedWith(
+      /Cannot deploy a web framework from source because the experiment.+webframeworks.+is not enabled/,
+    );
+  });
+});

src/deploy/index.ts

@@ -17,13 +17,15 @@
 import * as RemoteConfigTarget from "./remoteconfig";
 import * as ExtensionsTarget from "./extensions";
 import * as DataConnectTarget from "./dataconnect";
+import { HostingConfig } from "../firebaseConfig";
 import { prepareFrameworks } from "../frameworks";
-import { HostingDeploy } from "./hosting/context";
+import { FrameworkContext } from "../frameworks/interfaces";
 import { addPinnedFunctionsToOnlyString, hasPinnedFunctions } from "./hosting/prepare";
 import { isRunningInGithubAction } from "../init/features/hosting/github";
 import { TARGET_PERMISSIONS } from "../commands/deploy";
 import { requirePermissions } from "../requirePermissions";
 import { Options } from "../options";
+import { Context } from "./hosting/context";
 
 const TARGETS = {
   hosting: HostingTarget,
@@ -38,14 +40,37 @@
 
 export type DeployOptions = Options & { dryRun?: boolean };
 
 type Chain = ((context: any, options: any, payload: any) => Promise<unknown>)[];
 
 const chain = async function (fns: Chain, context: any, options: any, payload: any): Promise<void> {
   for (const latest of fns) {
     await latest(context, options, payload);
   }
 };
 
+export const matchesHostingTarget = (only: string | undefined, target?: string): boolean => {
+  if (!only) return true;
+  if (!only.includes("hosting:")) return true;
+  const targetStr = `hosting:${target ?? ""}`;
+  return only.split(",").some((t) => t === targetStr);
+};
+
+export const prepareFrameworksIfNeeded = async function (
+  targetNames: (keyof typeof TARGETS)[],
+  options: DeployOptions,
+  context: FrameworkContext,
+): Promise<void> {
+  const config = options.config.get("hosting") as HostingConfig;
+  if (
+    Array.isArray(config)
+      ? config.some((it) => it.source && matchesHostingTarget(options.only, it.target))
+      : config.source
+  ) {
+    experiments.assertEnabled("webframeworks", "deploy a web framework from source");
+    await prepareFrameworks("deploy", targetNames, context, options);
+  }
+};
+
 /**
  * The `deploy()` function runs through a three step deploy process for a listed
  * number of deploy targets. This allows deploys to be done all together or
@@ -59,7 +84,7 @@
   const projectId = needProjectId(options);
   const payload = {};
   // a shared context object for deploy targets to decorate as needed
-  const context: any = Object.assign({ projectId }, customContext);
+  const context: Context = Object.assign({ projectId }, customContext);
   const predeploys: Chain = [];
   const prepares: Chain = [];
   const deploys: Chain = [];
@@ -68,11 +93,7 @@
   const startTime = Date.now();
 
   if (targetNames.includes("hosting")) {
-    const config = options.config.get("hosting");
-    if (Array.isArray(config) ? config.some((it) => it.source) : config.source) {
-      experiments.assertEnabled("webframeworks", "deploy a web framework from source");
-      await prepareFrameworks("deploy", targetNames, context, options);
-    }
+    await prepareFrameworksIfNeeded(targetNames, options, context);
   }
 
   if (targetNames.includes("hosting") && hasPinnedFunctions(options)) {
@@ -148,11 +169,11 @@
   const deployedHosting = includes(targetNames, "hosting");
   logger.info(bold("Project Console:"), consoleUrl(options.project ?? "_", "/overview"));
   if (deployedHosting) {
-    each(context.hosting.deploys as HostingDeploy[], (deploy) => {
+    each(context.hosting?.deploys, (deploy) => {
       logger.info(bold("Hosting URL:"), addSubdomain(hostingOrigin(), deploy.config.site));
     });
-    const versionNames = context.hosting.deploys.map((deploy: any) => deploy.version);
-    return { hosting: versionNames.length === 1 ? versionNames[0] : versionNames };
+    const versionNames = context.hosting?.deploys.map((deploy: any) => deploy.version);
+    return { hosting: versionNames?.length === 1 ? versionNames[0] : versionNames };
   } else {
     return { hosting: undefined };
   }