fix: #8168 - enforce webframeworks only when needed

CHANGELOG.md

@@ -1,2 +1,3 @@
 - Added code generation of React hooks for Data Connect
 - Genkit init improvements around gcloud login and flow input values.
+- Enforce webframeworks enablement only on webframeworks sites (#8168)

src/deploy/hosting/deploy.spec.ts

@@ -0,0 +1,104 @@
+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 { 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();
+  });
+
+  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 StorageTarget from "./storage";
 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,
@@ -46,6 +48,27 @@ const chain = async function (fns: Chain, context: any, options: any, payload: a
   }
 };
 
+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 &&
+            (!options.only.includes("hosting:") ||
+              new RegExp(`\\bhosting:${it.target ?? ""}\\b`).exec(options.only)),
+        )
+      : 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 +82,7 @@ export const deploy = async function (
   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 +91,7 @@ export const deploy = async function (
   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 +167,11 @@ export const deploy = async function (
   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 };
   }