Skip to content

Commit

Permalink
Fix bug where CLI was unable to deploy Firebase Functions in some mon…
Browse files Browse the repository at this point in the history
…orepo setups (#5391)

Partially fixes #4952.

#5215 made changes to the CLI to detect Functions SDK even in monorepo setups where Functions SDK is hoisted (i.e. Functions SDK dependency is declared in the parent directory but not in the sub-package directory).

We should've made corresponding change in how the Functions SDK binary is executed - instead of always looking up the sub-package's `node_modules`, we should be looking at the `node_modules` closest to where the Functions SDK dependency is declared.

This setup seems common in scenarios where the developer bundles the functions source using bundlers like vite/webpack/etc. This kind of technique has shown to help [AWS lambda's cold start time](https://aws.amazon.com/blogs/compute/optimizing-node-js-dependencies-in-aws-lambda/), and I think it's something we'd want to explore in Google Cloud Functions too.
  • Loading branch information
taeold committed Jan 6, 2023
1 parent 68bdfa7 commit 9c2eeba
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix bug where CLI was unable to deploy Firebase Functions in some monorepo setups (#5391)
20 changes: 20 additions & 0 deletions scripts/functions-discover-tests/fixtures/bundled/dist/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Minimal example of a "bundled" function source.
*
* Instead of actually bundling the source code, we manually annotate
* the exported function with the __endpoint property to test the situation
* where the distributed package doesn't include Firebase Functions SDK as a
* dependency.
*/

const hello = (req, resp) => {
resp.send("hello");
};

hello.__endpoint = {
platform: "gcfv2",
region: "region",
httpsTrigger: {},
};

exports.hello = hello;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "dist",
"version": "0.0.1",
"engines": {
"node": "16"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"functions": {
"source": "dist"
}
}
5 changes: 5 additions & 0 deletions scripts/functions-discover-tests/fixtures/bundled/install.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/bash
set -euxo pipefail # bash strict mode
IFS=$'\n\t'

npm i
10 changes: 10 additions & 0 deletions scripts/functions-discover-tests/fixtures/bundled/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "dist",
"version": "0.0.1",
"dependencies": {
"firebase-functions": "^4.1.1"
},
"engines": {
"node": "16"
}
}
10 changes: 10 additions & 0 deletions scripts/functions-discover-tests/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ describe("Function discovery test", function (this) {
},
],
},
{
name: "bundled",
projectDir: "bundled",
expects: [
{
codebase: "default",
endpoints: ["hello"],
},
],
},
];

for (const tc of testCases) {
Expand Down
7 changes: 6 additions & 1 deletion src/deploy/functions/runtimes/node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ export class Delegate {
if (Object.keys(config || {}).length) {
env.CLOUD_RUNTIME_CONFIG = JSON.stringify(config);
}
const childProcess = spawn("./node_modules/.bin/firebase-functions", [this.sourceDir], {
// At this point, we've already confirmed that we found supported firebase functions sdk.
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"],
Expand Down

0 comments on commit 9c2eeba

Please sign in to comment.