From 1eed338e0a4e200ac1ec021f57be52669fd04bed Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 10 Oct 2022 16:02:34 -0700 Subject: [PATCH 1/6] Print a warning when Expression.value() is invoked during discovery --- src/params/types.ts | 41 ++++++++++++++++++++++++++++------------- src/runtime/loader.ts | 2 ++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/params/types.ts b/src/params/types.ts index ee9d40565..c2e9b8723 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -20,6 +20,8 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +import * as logger from "../logger"; + /* * A CEL expression which can be evaluated during function deployment, and * resolved to a value of the generic type parameter: i.e, you can pass @@ -28,6 +30,19 @@ export abstract class Expression { // Returns the Expression's runtime value, based on the CLI's resolution of params. value(): T { + if (process.env.FIREBASE_DISCOVERY_DIR) { + logger.warn( + `Parametrized expression ${this.toString()}.value() invoked during Function discovery. This is usually a mistake.` + ); + logger.warn( + `To configure a Function with an expression object, use it directly without calling .value().` + ); + } + return this.runtimeValue(); + } + + // @internal + runtimeValue(): T { throw new Error("Not implemented"); } @@ -47,7 +62,7 @@ function quoteIfString(literal: } function valueOf(arg: T | Expression): T { - return arg instanceof Expression ? arg.value() : arg; + return arg instanceof Expression ? arg.runtimeValue() : arg; } function refOf(arg: T | Expression): string { return arg instanceof Expression ? arg.toString() : quoteIfString(arg).toString(); @@ -69,8 +84,8 @@ export class TernaryExpression< this.ifFalse = ifFalse; } - value(): T { - return this.test.value() ? valueOf(this.ifTrue) : valueOf(this.ifFalse); + runtimeValue(): T { + return this.test.runtimeValue() ? valueOf(this.ifTrue) : valueOf(this.ifFalse); } toString() { @@ -100,8 +115,8 @@ export class CompareExpression< this.rhs = rhs; } - value(): boolean { - const left = this.lhs.value(); + runtimeValue(): boolean { + const left = this.lhs.runtimeValue(); const right = valueOf(this.rhs); switch (this.cmp) { case "==": @@ -210,7 +225,7 @@ export abstract class Param exte super(); } - value(): T { + runtimeValue(): T { throw new Error("Not implemented"); } @@ -277,7 +292,7 @@ export class SecretParam { this.name = name; } - value(): string { + runtimeValue(): string { return process.env[this.name] || ""; } @@ -290,7 +305,7 @@ export class SecretParam { } export class StringParam extends Param { - value(): string { + runtimeValue(): string { return process.env[this.name] || ""; } } @@ -307,7 +322,7 @@ export class InternalExpression extends Param { super(name); } - value(): string { + runtimeValue(): string { return this.getter(process.env) || ""; } @@ -319,7 +334,7 @@ export class InternalExpression extends Param { export class IntParam extends Param { static type: ParamValueType = "int"; - value(): number { + runtimeValue(): number { return parseInt(process.env[this.name] || "0", 10) || 0; } } @@ -327,7 +342,7 @@ export class IntParam extends Param { export class FloatParam extends Param { static type: ParamValueType = "float"; - value(): number { + runtimeValue(): number { return parseFloat(process.env[this.name] || "0") || 0; } } @@ -335,7 +350,7 @@ export class FloatParam extends Param { export class BooleanParam extends Param { static type: ParamValueType = "boolean"; - value(): boolean { + runtimeValue(): boolean { return !!process.env[this.name] && process.env[this.name] === "true"; } @@ -347,7 +362,7 @@ export class BooleanParam extends Param { export class ListParam extends Param { static type: ParamValueType = "list"; - value(): string[] { + runtimeValue(): string[] { throw new Error("Not implemented"); } diff --git a/src/runtime/loader.ts b/src/runtime/loader.ts index d1f9f2f32..ac84483a6 100644 --- a/src/runtime/loader.ts +++ b/src/runtime/loader.ts @@ -101,7 +101,9 @@ export async function loadStack(functionsDir: string): Promise { const requiredAPIs: ManifestRequiredAPI[] = []; const mod = await loadModule(functionsDir); + process.env.FIREBASE_DISCOVERY_DIR = functionsDir; extractStack(mod, endpoints, requiredAPIs); + delete process.env.FIREBASE_DISCOVERY_DIR; const stack: ManifestStack = { endpoints, From 5ce822013087c02d60cfa8cca66d4af8fbf99774 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 10 Oct 2022 16:21:22 -0700 Subject: [PATCH 2/6] Update src/params/types.ts Co-authored-by: Daniel Lee --- src/params/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/params/types.ts b/src/params/types.ts index c2e9b8723..a4daab616 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -35,7 +35,7 @@ export abstract class Expression `Parametrized expression ${this.toString()}.value() invoked during Function discovery. This is usually a mistake.` ); logger.warn( - `To configure a Function with an expression object, use it directly without calling .value().` + `To configure a function with a parameter, use it directly without calling .value().` ); } return this.runtimeValue(); From 60410a167003966b26ed177132b87075787cb5f2 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 10 Oct 2022 16:30:22 -0700 Subject: [PATCH 3/6] reworded message for clarity --- src/params/types.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/params/types.ts b/src/params/types.ts index a4daab616..edadc0d33 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -32,11 +32,12 @@ export abstract class Expression value(): T { if (process.env.FIREBASE_DISCOVERY_DIR) { logger.warn( - `Parametrized expression ${this.toString()}.value() invoked during Function discovery. This is usually a mistake.` + `${this.toString()}.value() invoked during Function configuration, instead of during runtime.` ); logger.warn( - `To configure a function with a parameter, use it directly without calling .value().` + `This is usually a mistake. In configs, use Params directly without calling .value().` ); + logger.warn(`\texample: { memory: memoryParam } not { memory: memoryParam.value() }`); } return this.runtimeValue(); } From a08a170886ab8e95b69ee7a2d1e3097d95e26fdd Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 10 Oct 2022 16:49:22 -0700 Subject: [PATCH 4/6] internal needs to be jsdoc format --- src/params/types.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/params/types.ts b/src/params/types.ts index edadc0d33..4d0360b78 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -42,7 +42,7 @@ export abstract class Expression return this.runtimeValue(); } - // @internal + /** @internal */ runtimeValue(): T { throw new Error("Not implemented"); } @@ -85,6 +85,7 @@ export class TernaryExpression< this.ifFalse = ifFalse; } + /** @internal */ runtimeValue(): T { return this.test.runtimeValue() ? valueOf(this.ifTrue) : valueOf(this.ifFalse); } @@ -116,6 +117,7 @@ export class CompareExpression< this.rhs = rhs; } + /** @internal */ runtimeValue(): boolean { const left = this.lhs.runtimeValue(); const right = valueOf(this.rhs); @@ -226,6 +228,7 @@ export abstract class Param exte super(); } + /** @internal */ runtimeValue(): T { throw new Error("Not implemented"); } @@ -293,6 +296,7 @@ export class SecretParam { this.name = name; } + /** @internal */ runtimeValue(): string { return process.env[this.name] || ""; } @@ -306,6 +310,7 @@ export class SecretParam { } export class StringParam extends Param { + /** @internal */ runtimeValue(): string { return process.env[this.name] || ""; } @@ -323,6 +328,7 @@ export class InternalExpression extends Param { super(name); } + /** @internal */ runtimeValue(): string { return this.getter(process.env) || ""; } @@ -335,6 +341,7 @@ export class InternalExpression extends Param { export class IntParam extends Param { static type: ParamValueType = "int"; + /** @internal */ runtimeValue(): number { return parseInt(process.env[this.name] || "0", 10) || 0; } @@ -343,6 +350,7 @@ export class IntParam extends Param { export class FloatParam extends Param { static type: ParamValueType = "float"; + /** @internal */ runtimeValue(): number { return parseFloat(process.env[this.name] || "0") || 0; } @@ -351,6 +359,7 @@ export class FloatParam extends Param { export class BooleanParam extends Param { static type: ParamValueType = "boolean"; + /** @internal */ runtimeValue(): boolean { return !!process.env[this.name] && process.env[this.name] === "true"; } @@ -363,6 +372,7 @@ export class BooleanParam extends Param { export class ListParam extends Param { static type: ParamValueType = "list"; + /** @internal */ runtimeValue(): string[] { throw new Error("Not implemented"); } From bf724c47d255e00a3ca5bed7ef3e9006cc226437 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 12 Oct 2022 12:38:03 -0700 Subject: [PATCH 5/6] Update src/params/types.ts --- src/params/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/params/types.ts b/src/params/types.ts index 3e2f21af0..bd5c1d72e 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -32,7 +32,7 @@ export abstract class Expression value(): T { if (process.env.FIREBASE_DISCOVERY_DIR) { logger.warn( - `${this.toString()}.value() invoked during Function configuration, instead of during runtime.` + `${this.toString()}.value() invoked during function deployment, instead of during runtime.` ); logger.warn( `This is usually a mistake. In configs, use Params directly without calling .value().` From 306a4466e50c4176e1e8a5874758334987ebe4e6 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 12 Oct 2022 13:19:50 -0700 Subject: [PATCH 6/6] hang off of env.FUNCTIONS_CONTROL_API instead --- src/params/types.ts | 4 ++-- src/runtime/loader.ts | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/params/types.ts b/src/params/types.ts index bd5c1d72e..5aafb2e27 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -30,14 +30,14 @@ import * as logger from "../logger"; export abstract class Expression { /** Returns the Expression's runtime value, based on the CLI's resolution of params. */ value(): T { - if (process.env.FIREBASE_DISCOVERY_DIR) { + if (process.env.FUNCTIONS_CONTROL_API === "true") { logger.warn( `${this.toString()}.value() invoked during function deployment, instead of during runtime.` ); logger.warn( `This is usually a mistake. In configs, use Params directly without calling .value().` ); - logger.warn(`\texample: { memory: memoryParam } not { memory: memoryParam.value() }`); + logger.warn(`example: { memory: memoryParam } not { memory: memoryParam.value() }`); } return this.runtimeValue(); } diff --git a/src/runtime/loader.ts b/src/runtime/loader.ts index ac84483a6..d1f9f2f32 100644 --- a/src/runtime/loader.ts +++ b/src/runtime/loader.ts @@ -101,9 +101,7 @@ export async function loadStack(functionsDir: string): Promise { const requiredAPIs: ManifestRequiredAPI[] = []; const mod = await loadModule(functionsDir); - process.env.FIREBASE_DISCOVERY_DIR = functionsDir; extractStack(mod, endpoints, requiredAPIs); - delete process.env.FIREBASE_DISCOVERY_DIR; const stack: ManifestStack = { endpoints,