Skip to content

Commit

Permalink
Fix bug where function discovery failed on projects using yarn worksp…
Browse files Browse the repository at this point in the history
…aces. (#5215)
  • Loading branch information
taeold committed Dec 13, 2022
1 parent 7bcbf8e commit 355e9b9
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"functions": {
"source": "packages/functions"
}
}
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'

yarn install
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "yarn-workspace",
"private": true,
"workspaces": ["packages/functions", "packages/a"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.msg = "Hello world!";
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "a",
"version": "0.0.1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const functions = require("firebase-functions");
const { onRequest } = require("firebase-functions/v2/https");
const { msg } = require("a");

exports.hellov1 = functions.https.onRequest((request, response) => {
response.send(msg);
});

exports.hellov2 = onRequest((request, response) => {
response.send(msg);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "simple",
"version": "0.0.1",
"dependencies": {
"firebase-functions": "4.0.0",
"firebase-admin": "^11.2.0",
"a": "latest"
},
"engines": {
"node": "16"
}
}
3 changes: 3 additions & 0 deletions scripts/functions-discover-tests/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ IFS=$'\n\t'
# Unlock internal commands for discovering functions in a project.
firebase experiments:enable internaltesting

# Install yarn
npm i -g yarn

for dir in ./scripts/functions-discover-tests/fixtures/*; do
(cd $dir && ./install.sh)
done
Expand Down
10 changes: 10 additions & 0 deletions scripts/functions-discover-tests/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ describe("Function discovery test", function (this) {
},
],
},
{
name: "yarn-workspaces",
projectDir: "yarn-workspaces",
expects: [
{
codebase: "default",
endpoints: ["hellov1", "hellov2"],
},
],
},
];

for (const tc of testCases) {
Expand Down
4 changes: 2 additions & 2 deletions src/deploy/functions/runtimes/node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ export class Delegate {
// Using a caching interface because we (may/will) eventually depend on the SDK version
// to decide whether to use the JS export method of discovery or the HTTP container contract
// method of discovery.
_sdkVersion = "";
_sdkVersion: string | undefined = undefined;
get sdkVersion() {
if (!this._sdkVersion) {
if (this._sdkVersion === undefined) {
this._sdkVersion = versioning.getFunctionsSDKVersion(this.sourceDir) || "";
}
return this._sdkVersion;
Expand Down
77 changes: 52 additions & 25 deletions src/deploy/functions/runtimes/node/versioning.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import * as _ from "lodash";
import * as fs from "fs";
import * as path from "path";

import * as clc from "colorette";
import * as semver from "semver";
import * as spawn from "cross-spawn";
Expand All @@ -7,17 +9,6 @@ import * as utils from "../../../../utils";
import { logger } from "../../../../logger";
import { track } from "../../../../track";

interface NpmListResult {
name: string;
dependencies: {
"firebase-functions": {
version: string;
from: string;
resolved: string;
};
};
}

interface NpmShowResult {
"dist-tags": {
latest: string;
Expand All @@ -34,30 +25,66 @@ export const FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING =
clc.bold("npm i --save firebase-functions@latest") +
" in the functions folder.";

/**
* Exported for testing purposes only.
*
* @internal
*/
export function findModuleVersion(name: string, resolvedPath: string): string | undefined {
let searchPath = path.dirname(resolvedPath);
// eslint-disable-next-line no-constant-condition
while (true) {
if (searchPath === "/" || path.basename(searchPath) === "node_modules") {
logger.debug(
`Failed to find version of module ${name}: reached end of search path ${searchPath}`
);
return;
}
const maybePackageJson = path.join(searchPath, "package.json");
if (fs.existsSync(maybePackageJson)) {
const pkg = require(maybePackageJson);
if (pkg.name === name) {
return pkg.version;
}
logger.debug(
`Failed to find version of module ${name}: instead found ${pkg.name} at ${searchPath}`
);
return;
}
searchPath = path.dirname(searchPath);
}
}

/**
* Returns the version of firebase-functions SDK specified by package.json and package-lock.json.
* @param sourceDir Source directory of functions code
* @return version string (e.g. "3.1.2"), or void if firebase-functions is not in package.json
* or if we had trouble getting the version.
*/
export function getFunctionsSDKVersion(sourceDir: string): string | void {
export function getFunctionsSDKVersion(sourceDir: string): string | undefined {
try {
const child = spawn.sync("npm", ["list", "firebase-functions", "--json=true"], {
cwd: sourceDir,
encoding: "utf8",
});
if (child.error) {
logger.debug("getFunctionsSDKVersion encountered error:", child.error.stack);
return;
}
const output: NpmListResult = JSON.parse(child.stdout);
return _.get(output, ["dependencies", "firebase-functions", "version"]);
return findModuleVersion(
"firebase-functions",
// Find the entry point of the firebase-function module. require.resolve works for project directories using
// npm, yarn (1), or yarn (1) workspaces. Does not support yarn (2) since GCF doesn't support it anyway:
// https://issuetracker.google.com/issues/213632942.
require.resolve("firebase-functions", { paths: [sourceDir] })
);
} catch (e: any) {
if (e.code === "MODULE_NOT_FOUND") {
utils.logLabeledWarning(
"functions",
"Couldn't find firebase-functions package in your source code. Have you run 'npm install'?"
);
}
logger.debug("getFunctionsSDKVersion encountered error:", e);
return;
}
}

/**
* Get latest version of the Firebase Functions SDK.
*/
export function getLatestSDKVersion(): string | undefined {
const child = spawn.sync("npm", ["show", "firebase-functions", "--json=true"], {
encoding: "utf8",
Expand All @@ -70,10 +97,10 @@ export function getLatestSDKVersion(): string | undefined {
return;
}
const output: NpmShowResult = JSON.parse(child.stdout);
if (_.isEmpty(output)) {
if (Object.keys(output).length === 0) {
return;
}
return _.get(output, ["dist-tags", "latest"]);
return output["dist-tags"]?.["latest"];
}

/**
Expand Down

0 comments on commit 355e9b9

Please sign in to comment.