Fix broken pnpm support for cf3 (#5467)

Due to the way pnpm resolves packages with peer dependencies, location of the firebase functions sdk package isn't where the CLI expects it to be.

We make the logic for finding the binary associated Firebase Functions SDK more robust by checking list of possible paths where it might exist. We also add integration test for pnpm in the functions discovery test.

Fixes #5448

Author: taeold

CHANGELOG.md

@@ -3,3 +3,4 @@
 - Fixes issue where `init firestore` was unecessarilly checking for default resource location. (#5230 and #5452)
 - Pass `trailingSlash` from Next.js config to `firebase.json` (#5445)
 - Don't use Next.js internal redirects for the backend test (#5445)
+- Fix issue where pnpm support broke for function emulation and deployment. (#5467)

scripts/functions-discover-tests/fixtures/pnpm/firebase.json

@@ -0,0 +1 @@
+{}

scripts/functions-discover-tests/fixtures/pnpm/functions/index.js

@@ -0,0 +1,10 @@
+const functions = require("firebase-functions");
+const { onRequest } = require("firebase-functions/v2/https");
+
+exports.hellov1 = functions.https.onRequest((request, response) => {
+  response.send("Hello from Firebase!");
+});
+
+exports.hellov2 = onRequest((request, response) => {
+  response.send("Hello from Firebase!");
+});

scripts/functions-discover-tests/fixtures/pnpm/functions/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "pnpm",
+  "dependencies": {
+    "firebase-functions": "^4.0.0"
+  },
+  "engines": {
+    "node": "16"
+  }
+}

scripts/functions-discover-tests/fixtures/pnpm/install.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+set -euxo pipefail # bash strict mode
+IFS=$'\n\t'
+
+cd functions && pnpm install

scripts/functions-discover-tests/run.sh

@@ -11,6 +11,9 @@ firebase experiments:enable internaltesting
 # Install yarn
 npm i -g yarn
 
+# Install pnpm
+npm install -g pnpm --force # it's okay to reinstall pnpm
+
 for dir in ./scripts/functions-discover-tests/fixtures/*; do
   (cd $dir && ./install.sh)
 done

scripts/functions-discover-tests/tests.ts

@@ -77,6 +77,16 @@ describe("Function discovery test", function (this) {
         },
       ],
     },
+    {
+      name: "pnpm",
+      projectDir: "pnpm",
+      expects: [
+        {
+          codebase: "default",
+          endpoints: ["hellov1", "hellov2"],
+        },
+      ],
+    },
   ];
 
   for (const tc of testCases) {

src/deploy/functions/runtimes/node/index.ts

@@ -17,6 +17,7 @@ import * as runtimes from "..";
 import * as validate from "./validate";
 import * as versioning from "./versioning";
 import * as parseTriggers from "./parseTriggers";
+import { fileExistsSync } from "../../../../fsutils";
 
 const MIN_FUNCTIONS_SDK_VERSION = "3.20.0";
 
@@ -169,33 +170,51 @@ export class Delegate {
     if (Object.keys(config || {}).length) {
       env.CLOUD_RUNTIME_CONFIG = JSON.stringify(config);
     }
-    // At this point, we've already confirmed that we found supported firebase functions sdk.
+    // Location of the binary included in the Firebase Functions SDK
+    // differs depending on the developer's setup and choice of package manager.
+    //
+    // We'll try few routes in the following order:
+    //
+    //   1. $SOURCE_DIR/node_modules/.bin/firebase-functions
+    //   2. node_modules closest to the resolved path ${require.resolve("firebase-functions")}
+    //
+    // (1) works for most package managers (npm, yarn[no-hoist],pnpm).
+    // (2) handles cases where developer prefers monorepo setup or bundled function code.
+    const sourceNodeModulesPath = path.join(this.sourceDir, "node_modules");
     const sdkPath = require.resolve("firebase-functions", { paths: [this.sourceDir] });
-    // Find location of the closest node_modules/ directory where we found the sdk.
-    const binPath = sdkPath.substring(0, sdkPath.lastIndexOf("node_modules") + 12);
-    // And execute the binary included in the sdk.
-    const childProcess = spawn(path.join(binPath, ".bin", "firebase-functions"), [this.sourceDir], {
-      env,
-      cwd: this.sourceDir,
-      stdio: [/* stdin=*/ "ignore", /* stdout=*/ "pipe", /* stderr=*/ "inherit"],
-    });
-    childProcess.stdout?.on("data", (chunk) => {
-      logger.debug(chunk.toString());
-    });
-    return Promise.resolve(async () => {
-      const p = new Promise<void>((resolve, reject) => {
-        childProcess.once("exit", resolve);
-        childProcess.once("error", reject);
-      });
-
-      await fetch(`http://localhost:${port}/__/quitquitquit`);
-      setTimeout(() => {
-        if (!childProcess.killed) {
-          childProcess.kill("SIGKILL");
-        }
-      }, 10_000);
-      return p;
-    });
+    const sdkNodeModulesPath = sdkPath.substring(0, sdkPath.lastIndexOf("node_modules") + 12);
+    for (const nodeModulesPath of [sourceNodeModulesPath, sdkNodeModulesPath]) {
+      const binPath = path.join(nodeModulesPath, ".bin", "firebase-functions");
+      if (fileExistsSync(binPath)) {
+        logger.debug(`Found firebase-functions binary at '${binPath}'`);
+        const childProcess = spawn(binPath, [this.sourceDir], {
+          env,
+          cwd: this.sourceDir,
+          stdio: [/* stdin=*/ "ignore", /* stdout=*/ "pipe", /* stderr=*/ "inherit"],
+        });
+        childProcess.stdout?.on("data", (chunk) => {
+          logger.debug(chunk.toString());
+        });
+        return Promise.resolve(async () => {
+          const p = new Promise<void>((resolve, reject) => {
+            childProcess.once("exit", resolve);
+            childProcess.once("error", reject);
+          });
+
+          await fetch(`http://localhost:${port}/__/quitquitquit`);
+          setTimeout(() => {
+            if (!childProcess.killed) {
+              childProcess.kill("SIGKILL");
+            }
+          }, 10_000);
+          return p;
+        });
+      }
+    }
+    throw new FirebaseError(
+      "Failed to find location of Firebase Functions SDK. " +
+        "Please file a bug on Github (https://github.com/firebase/firebase-tools/)."
+    );
   }
 
   // eslint-disable-next-line require-await