-
Notifications
You must be signed in to change notification settings - Fork 213
Print a warning when Expression.value() is invoked during discovery #1257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
1eed338
5ce8220
60410a1
a08a170
b7a314d
bf724c4
306a446
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,20 @@ | |
export abstract class Expression<T extends string | number | boolean | string[]> { | ||
// Returns the Expression's runtime value, based on the CLI's resolution of params. | ||
value(): T { | ||
if (process.env.FIREBASE_DISCOVERY_DIR) { | ||
logger.warn( | ||
`${this.toString()}.value() invoked during Function configuration, 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() }`); | ||
} | ||
return this.runtimeValue(); | ||
} | ||
|
||
/** @internal */ | ||
runtimeValue(): T { | ||
throw new Error("Not implemented"); | ||
} | ||
|
||
|
@@ -47,7 +63,7 @@ function quoteIfString<T extends string | number | boolean | string[]>(literal: | |
} | ||
|
||
function valueOf<T extends string | number | boolean | string[]>(arg: T | Expression<T>): T { | ||
return arg instanceof Expression ? arg.value() : arg; | ||
return arg instanceof Expression ? arg.runtimeValue() : arg; | ||
} | ||
function refOf<T extends string | number | boolean | string[]>(arg: T | Expression<T>): string { | ||
return arg instanceof Expression ? arg.toString() : quoteIfString(arg).toString(); | ||
|
@@ -69,8 +85,9 @@ export class TernaryExpression< | |
this.ifFalse = ifFalse; | ||
} | ||
|
||
value(): T { | ||
return this.test.value() ? valueOf(this.ifTrue) : valueOf(this.ifFalse); | ||
/** @internal */ | ||
runtimeValue(): T { | ||
return this.test.runtimeValue() ? valueOf(this.ifTrue) : valueOf(this.ifFalse); | ||
Comment on lines
+89
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. woah we are renaming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not from users' perspective. They'll still call value(), which will use the implementation in the base Expression class, which will do the discovery check and then call runtimeValue() |
||
} | ||
|
||
toString() { | ||
|
@@ -100,8 +117,9 @@ export class CompareExpression< | |
this.rhs = rhs; | ||
} | ||
|
||
value(): boolean { | ||
const left = this.lhs.value(); | ||
/** @internal */ | ||
runtimeValue(): boolean { | ||
const left = this.lhs.runtimeValue(); | ||
const right = valueOf(this.rhs); | ||
switch (this.cmp) { | ||
case "==": | ||
|
@@ -210,7 +228,8 @@ export abstract class Param<T extends string | number | boolean | string[]> exte | |
super(); | ||
} | ||
|
||
value(): T { | ||
/** @internal */ | ||
runtimeValue(): T { | ||
throw new Error("Not implemented"); | ||
} | ||
|
||
|
@@ -277,7 +296,8 @@ export class SecretParam { | |
this.name = name; | ||
} | ||
|
||
value(): string { | ||
/** @internal */ | ||
runtimeValue(): string { | ||
return process.env[this.name] || ""; | ||
} | ||
|
||
|
@@ -290,7 +310,8 @@ export class SecretParam { | |
} | ||
|
||
export class StringParam extends Param<string> { | ||
value(): string { | ||
/** @internal */ | ||
runtimeValue(): string { | ||
return process.env[this.name] || ""; | ||
} | ||
} | ||
|
@@ -307,7 +328,8 @@ export class InternalExpression extends Param<string> { | |
super(name); | ||
} | ||
|
||
value(): string { | ||
/** @internal */ | ||
runtimeValue(): string { | ||
return this.getter(process.env) || ""; | ||
} | ||
|
||
|
@@ -319,23 +341,26 @@ export class InternalExpression extends Param<string> { | |
export class IntParam extends Param<number> { | ||
static type: ParamValueType = "int"; | ||
|
||
value(): number { | ||
/** @internal */ | ||
runtimeValue(): number { | ||
return parseInt(process.env[this.name] || "0", 10) || 0; | ||
} | ||
} | ||
|
||
export class FloatParam extends Param<number> { | ||
static type: ParamValueType = "float"; | ||
|
||
value(): number { | ||
/** @internal */ | ||
runtimeValue(): number { | ||
return parseFloat(process.env[this.name] || "0") || 0; | ||
} | ||
} | ||
|
||
export class BooleanParam extends Param<boolean> { | ||
static type: ParamValueType = "boolean"; | ||
|
||
value(): boolean { | ||
/** @internal */ | ||
runtimeValue(): boolean { | ||
return !!process.env[this.name] && process.env[this.name] === "true"; | ||
} | ||
|
||
|
@@ -347,7 +372,8 @@ export class BooleanParam extends Param<boolean> { | |
export class ListParam extends Param<string[]> { | ||
static type: ParamValueType = "list"; | ||
|
||
value(): string[] { | ||
/** @internal */ | ||
runtimeValue(): string[] { | ||
throw new Error("Not implemented"); | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.