From 54e0e9cdb0de12665aed6599a1ab812f82555049 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 6 Jul 2022 17:00:42 -0700 Subject: [PATCH 1/5] allow users to present String and Integer params in a ManifestSpec --- .../fixtures/sources/commonjs-params/index.js | 21 +++++++++++++++++++ .../sources/commonjs-params/package.json | 3 +++ spec/runtime/loader.spec.ts | 5 +++++ src/runtime/loader.ts | 8 ++++++- src/runtime/manifest.ts | 3 +++ src/v2/params/index.ts | 10 +++++---- src/v2/params/types.ts | 20 +++++++++--------- 7 files changed, 55 insertions(+), 15 deletions(-) create mode 100644 spec/fixtures/sources/commonjs-params/index.js create mode 100644 spec/fixtures/sources/commonjs-params/package.json diff --git a/spec/fixtures/sources/commonjs-params/index.js b/spec/fixtures/sources/commonjs-params/index.js new file mode 100644 index 000000000..94afc2359 --- /dev/null +++ b/spec/fixtures/sources/commonjs-params/index.js @@ -0,0 +1,21 @@ +const functions = require("../../../../src/index"); +const functionsv2 = require("../../../../src/v2/index"); +const { defineString } = require("../../../../src/v2/params"); + +defineString("foo"); + +exports.v1http = functions.https.onRequest((req, resp) => { + resp.status(200).send("PASS"); +}); + +exports.v1callable = functions.https.onCall(() => { + return "PASS"; +}); + +exports.v2http = functionsv2.https.onRequest((req, resp) => { + resp.status(200).send("PASS"); +}); + +exports.v2callable = functionsv2.https.onCall(() => { + return "PASS"; +}); diff --git a/spec/fixtures/sources/commonjs-params/package.json b/spec/fixtures/sources/commonjs-params/package.json new file mode 100644 index 000000000..30e1b1b27 --- /dev/null +++ b/spec/fixtures/sources/commonjs-params/package.json @@ -0,0 +1,3 @@ +{ + "name": "commonjs" +} diff --git a/spec/runtime/loader.spec.ts b/spec/runtime/loader.spec.ts index 9994557c4..2ef3d9bf0 100644 --- a/spec/runtime/loader.spec.ts +++ b/spec/runtime/loader.spec.ts @@ -310,6 +310,11 @@ describe('loadStack', () => { }, }, }, + { + name: 'has params', + modulePath: './spec/fixtures/sources/commonjs-params', + expected: {...expected, params: [{"name": "foo", "type": "string"}]}, + }, ]; for (const tc of testcases) { diff --git a/src/runtime/loader.ts b/src/runtime/loader.ts index de31eefd3..c29fb9bb7 100644 --- a/src/runtime/loader.ts +++ b/src/runtime/loader.ts @@ -28,6 +28,8 @@ import { ManifestStack, } from './manifest'; +import * as params from "../v2/params"; + /** * Dynamically load import function to prevent TypeScript from * transpiling into a require. @@ -112,9 +114,13 @@ export async function loadStack(functionsDir: string): Promise { extractStack(mod, endpoints, requiredAPIs); - return { + var stack:ManifestStack = { endpoints, specVersion: 'v1alpha1', requiredAPIs: mergeRequiredAPIs(requiredAPIs), }; + if (params.declaredParams.length > 0) { + stack.params = params.declaredParams.map((p) => p.toSpec()); + } + return stack } diff --git a/src/runtime/manifest.ts b/src/runtime/manifest.ts index 562d1a52f..334474e64 100644 --- a/src/runtime/manifest.ts +++ b/src/runtime/manifest.ts @@ -20,6 +20,8 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +import {ParamSpec} from "../v2/params/types"; + /** * An definition of a function as appears in the Manifest. */ @@ -87,6 +89,7 @@ export interface ManifestRequiredAPI { */ export interface ManifestStack { specVersion: 'v1alpha1'; + params?: ParamSpec[]; requiredAPIs: ManifestRequiredAPI[]; endpoints: Record; } diff --git a/src/v2/params/index.ts b/src/v2/params/index.ts index bf45e8c45..b2b2f4f25 100644 --- a/src/v2/params/index.ts +++ b/src/v2/params/index.ts @@ -20,10 +20,6 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -/** - * @hidden - * @alpha - */ import { BooleanParam, FloatParam, @@ -56,6 +52,7 @@ function registerParam(param: Param) { /** * Declare a string param. * + * @alpha * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `string` return type for `.value`. @@ -72,6 +69,7 @@ export function defineString( /** * Declare a boolean param. * + * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `boolean` return type for `.value`. @@ -88,6 +86,7 @@ export function defineBoolean( /** * Declare an integer param. * + * @alpha * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `number` return type for `.value`. @@ -104,6 +103,7 @@ export function defineInt( /** * Declare a float param. * + * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `number` return type for `.value`. @@ -120,6 +120,7 @@ export function defineFloat( /** * Declare a list param (array of strings). * + * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `string[]` return type for `.value`. @@ -137,6 +138,7 @@ export function defineList( * Declare a JSON param. The associated environment variable will be treated * as a JSON string when loading its value. * + * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a specifiable return type for `.value`. diff --git a/src/v2/params/types.ts b/src/v2/params/types.ts index 9612dc8dc..1ec806551 100644 --- a/src/v2/params/types.ts +++ b/src/v2/params/types.ts @@ -28,16 +28,16 @@ export interface ParamSpec { default?: T; label?: string; description?: string; - valueType?: ParamValueType; + type: ParamValueType; } export type ParamOptions = Omit< ParamSpec, - 'name' | 'valueType' + 'name' | 'type' >; export class Param { - static valueType: ParamValueType = 'string'; + static type: ParamValueType = 'string'; constructor(readonly name: string, readonly options: ParamOptions = {}) {} @@ -61,7 +61,7 @@ export class Param { const out: ParamSpec = { name: this.name, ...this.options, - valueType: (this.constructor as typeof Param).valueType, + type: (this.constructor as typeof Param).type, }; if (this.options.default && typeof this.options.default !== 'string') { out.default = (this.options.default as @@ -78,7 +78,7 @@ export class StringParam extends Param { } export class IntParam extends Param { - static valueType: ParamValueType = 'int'; + static type: ParamValueType = 'int'; get value(): number { const intVal = parseInt( @@ -97,7 +97,7 @@ export class IntParam extends Param { } export class FloatParam extends Param { - static valueType: ParamValueType = 'float'; + static type: ParamValueType = 'float'; get value(): number { const floatVal = parseFloat( @@ -115,7 +115,7 @@ export class FloatParam extends Param { } export class BooleanParam extends Param { - static valueType: ParamValueType = 'boolean'; + static type: ParamValueType = 'boolean'; get value(): boolean { const lowerVal = ( @@ -137,7 +137,7 @@ export class BooleanParam extends Param { } export class ListParam extends Param { - static valueType: ParamValueType = 'list'; + static type: ParamValueType = 'list'; get value(): string[] { return typeof this.rawValue === 'string' @@ -148,7 +148,7 @@ export class ListParam extends Param { toSpec(): ParamSpec { const out: ParamSpec = { name: this.name, - valueType: 'list', + type: 'list', ...this.options, }; if (this.options.default && this.options.default.length > 0) { @@ -160,7 +160,7 @@ export class ListParam extends Param { } export class JSONParam extends Param { - static valueType: ParamValueType = 'json'; + static type: ParamValueType = 'json'; get value(): T { if (this.rawValue) { From a38bad78629d1d22a1a9a00833c0cebc2feb138c Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 6 Jul 2022 17:05:06 -0700 Subject: [PATCH 2/5] format:fix --- spec/runtime/loader.spec.ts | 2 +- src/runtime/loader.ts | 6 +++--- src/runtime/manifest.ts | 2 +- src/v2/params/types.ts | 5 +---- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/spec/runtime/loader.spec.ts b/spec/runtime/loader.spec.ts index 2ef3d9bf0..2514f7db4 100644 --- a/spec/runtime/loader.spec.ts +++ b/spec/runtime/loader.spec.ts @@ -313,7 +313,7 @@ describe('loadStack', () => { { name: 'has params', modulePath: './spec/fixtures/sources/commonjs-params', - expected: {...expected, params: [{"name": "foo", "type": "string"}]}, + expected: { ...expected, params: [{ name: 'foo', type: 'string' }] }, }, ]; diff --git a/src/runtime/loader.ts b/src/runtime/loader.ts index c29fb9bb7..f72476ecc 100644 --- a/src/runtime/loader.ts +++ b/src/runtime/loader.ts @@ -28,7 +28,7 @@ import { ManifestStack, } from './manifest'; -import * as params from "../v2/params"; +import * as params from '../v2/params'; /** * Dynamically load import function to prevent TypeScript from @@ -114,7 +114,7 @@ export async function loadStack(functionsDir: string): Promise { extractStack(mod, endpoints, requiredAPIs); - var stack:ManifestStack = { + var stack: ManifestStack = { endpoints, specVersion: 'v1alpha1', requiredAPIs: mergeRequiredAPIs(requiredAPIs), @@ -122,5 +122,5 @@ export async function loadStack(functionsDir: string): Promise { if (params.declaredParams.length > 0) { stack.params = params.declaredParams.map((p) => p.toSpec()); } - return stack + return stack; } diff --git a/src/runtime/manifest.ts b/src/runtime/manifest.ts index 334474e64..d63088303 100644 --- a/src/runtime/manifest.ts +++ b/src/runtime/manifest.ts @@ -20,7 +20,7 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -import {ParamSpec} from "../v2/params/types"; +import { ParamSpec } from '../v2/params/types'; /** * An definition of a function as appears in the Manifest. diff --git a/src/v2/params/types.ts b/src/v2/params/types.ts index 1ec806551..fc898c15c 100644 --- a/src/v2/params/types.ts +++ b/src/v2/params/types.ts @@ -31,10 +31,7 @@ export interface ParamSpec { type: ParamValueType; } -export type ParamOptions = Omit< - ParamSpec, - 'name' | 'type' ->; +export type ParamOptions = Omit, 'name' | 'type'>; export class Param { static type: ParamValueType = 'string'; From 6887a7265a0ce8969a61505737fbf7bc58140342 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 6 Jul 2022 17:06:26 -0700 Subject: [PATCH 3/5] fix bad copy-paste in fixture --- spec/fixtures/sources/commonjs-params/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fixtures/sources/commonjs-params/package.json b/spec/fixtures/sources/commonjs-params/package.json index 30e1b1b27..91f8c11da 100644 --- a/spec/fixtures/sources/commonjs-params/package.json +++ b/spec/fixtures/sources/commonjs-params/package.json @@ -1,3 +1,3 @@ { - "name": "commonjs" + "name": "commonjs-params" } From 6ab879cfd77ee916302de15b0fe1ab2f86b8cd2d Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 13 Jul 2022 17:25:00 -0700 Subject: [PATCH 4/5] fixes for code review, especially reverting changes to visibility in params/index.ts --- spec/fixtures/sources/commonjs-params/index.js | 2 +- spec/runtime/loader.spec.ts | 2 +- src/runtime/loader.ts | 2 +- src/v2/params/index.ts | 9 +++++++-- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/spec/fixtures/sources/commonjs-params/index.js b/spec/fixtures/sources/commonjs-params/index.js index 94afc2359..d038a2f77 100644 --- a/spec/fixtures/sources/commonjs-params/index.js +++ b/spec/fixtures/sources/commonjs-params/index.js @@ -2,7 +2,7 @@ const functions = require("../../../../src/index"); const functionsv2 = require("../../../../src/v2/index"); const { defineString } = require("../../../../src/v2/params"); -defineString("foo"); +defineString("FOO"); exports.v1http = functions.https.onRequest((req, resp) => { resp.status(200).send("PASS"); diff --git a/spec/runtime/loader.spec.ts b/spec/runtime/loader.spec.ts index 2514f7db4..bb959a3cb 100644 --- a/spec/runtime/loader.spec.ts +++ b/spec/runtime/loader.spec.ts @@ -313,7 +313,7 @@ describe('loadStack', () => { { name: 'has params', modulePath: './spec/fixtures/sources/commonjs-params', - expected: { ...expected, params: [{ name: 'foo', type: 'string' }] }, + expected: { ...expected, params: [{ name: 'FOO', type: 'string' }] }, }, ]; diff --git a/src/runtime/loader.ts b/src/runtime/loader.ts index f72476ecc..4dd4b41d2 100644 --- a/src/runtime/loader.ts +++ b/src/runtime/loader.ts @@ -114,7 +114,7 @@ export async function loadStack(functionsDir: string): Promise { extractStack(mod, endpoints, requiredAPIs); - var stack: ManifestStack = { + const stack: ManifestStack = { endpoints, specVersion: 'v1alpha1', requiredAPIs: mergeRequiredAPIs(requiredAPIs), diff --git a/src/v2/params/index.ts b/src/v2/params/index.ts index b2b2f4f25..daf01402b 100644 --- a/src/v2/params/index.ts +++ b/src/v2/params/index.ts @@ -20,6 +20,11 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +/** + * @hidden + * @alpha + */ + import { BooleanParam, FloatParam, @@ -52,7 +57,7 @@ function registerParam(param: Param) { /** * Declare a string param. * - * @alpha + * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `string` return type for `.value`. @@ -86,7 +91,7 @@ export function defineBoolean( /** * Declare an integer param. * - * @alpha + * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `number` return type for `.value`. From 8eb1b7806e19b024f4006c6d9f97bc7b82f3673e Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 13 Jul 2022 17:26:30 -0700 Subject: [PATCH 5/5] more visibility fixes --- src/v2/params/index.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/v2/params/index.ts b/src/v2/params/index.ts index daf01402b..3f8a080b6 100644 --- a/src/v2/params/index.ts +++ b/src/v2/params/index.ts @@ -57,7 +57,6 @@ function registerParam(param: Param) { /** * Declare a string param. * - * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `string` return type for `.value`. @@ -74,7 +73,6 @@ export function defineString( /** * Declare a boolean param. * - * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `boolean` return type for `.value`. @@ -91,7 +89,6 @@ export function defineBoolean( /** * Declare an integer param. * - * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `number` return type for `.value`. @@ -108,7 +105,6 @@ export function defineInt( /** * Declare a float param. * - * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `number` return type for `.value`. @@ -125,7 +121,6 @@ export function defineFloat( /** * Declare a list param (array of strings). * - * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a `string[]` return type for `.value`. @@ -143,7 +138,6 @@ export function defineList( * Declare a JSON param. The associated environment variable will be treated * as a JSON string when loading its value. * - * @hidden * @param name The name of the environment variable to use to load the param. * @param options Configuration options for the param. * @returns A Param with a specifiable return type for `.value`.