diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb..fcaa37c26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Fix "Dual-Package Hazard" for parameterized configuration in ESM projects. (#1780) diff --git a/scripts/bin-test/sources/commonjs-params/index.js b/scripts/bin-test/sources/commonjs-params/index.js new file mode 100644 index 000000000..3dac9f734 --- /dev/null +++ b/scripts/bin-test/sources/commonjs-params/index.js @@ -0,0 +1,8 @@ +const { defineInt } = require("firebase-functions/params"); +const { onRequest } = require("firebase-functions/v2/https"); + +const minInstances = defineInt("MIN_INSTANCES", { default: 1 }); + +exports.v2http = onRequest({ minInstances }, (req, res) => { + res.send("PASS"); +}); diff --git a/scripts/bin-test/sources/commonjs-params/package.json b/scripts/bin-test/sources/commonjs-params/package.json new file mode 100644 index 000000000..1bfd80923 --- /dev/null +++ b/scripts/bin-test/sources/commonjs-params/package.json @@ -0,0 +1,4 @@ +{ + "name": "commonjs-params", + "main": "index.js" +} diff --git a/scripts/bin-test/sources/esm-params/index.js b/scripts/bin-test/sources/esm-params/index.js new file mode 100644 index 000000000..2055b63a4 --- /dev/null +++ b/scripts/bin-test/sources/esm-params/index.js @@ -0,0 +1,8 @@ +import { defineInt } from "firebase-functions/params"; +import { onRequest } from "firebase-functions/v2/https"; + +const minInstances = defineInt("MIN_INSTANCES", { default: 1 }); + +export const v2http = onRequest({ minInstances }, (req, res) => { + res.send("PASS"); +}); diff --git a/scripts/bin-test/sources/esm-params/package.json b/scripts/bin-test/sources/esm-params/package.json new file mode 100644 index 000000000..2a9ed5519 --- /dev/null +++ b/scripts/bin-test/sources/esm-params/package.json @@ -0,0 +1,4 @@ +{ + "name": "esm-params", + "type": "module" +} diff --git a/scripts/bin-test/test.ts b/scripts/bin-test/test.ts index d24eec5cd..c22d6b00b 100644 --- a/scripts/bin-test/test.ts +++ b/scripts/bin-test/test.ts @@ -362,6 +362,32 @@ describe("functions.yaml", function () { extensions: BASE_EXTENSIONS, }, }, + { + name: "with params", + modulePath: "./scripts/bin-test/sources/commonjs-params", + expected: { + endpoints: { + v2http: { + ...DEFAULT_V2_OPTIONS, + platform: "gcfv2", + entryPoint: "v2http", + labels: {}, + httpsTrigger: {}, + minInstances: "{{ params.MIN_INSTANCES }}", + }, + }, + requiredAPIs: [], + specVersion: "v1alpha1", + params: [ + { + name: "MIN_INSTANCES", + type: "int", + default: 1, + }, + ], + extensions: {}, + }, + }, ]; for (const tc of testcases) { @@ -396,6 +422,32 @@ describe("functions.yaml", function () { modulePath: "./scripts/bin-test/sources/esm-ext", expected: BASE_STACK, }, + { + name: "with params", + modulePath: "./scripts/bin-test/sources/esm-params", + expected: { + endpoints: { + v2http: { + ...DEFAULT_V2_OPTIONS, + platform: "gcfv2", + entryPoint: "v2http", + labels: {}, + httpsTrigger: {}, + minInstances: "{{ params.MIN_INSTANCES }}", + }, + }, + requiredAPIs: [], + specVersion: "v1alpha1", + params: [ + { + name: "MIN_INSTANCES", + type: "int", + default: 1, + }, + ], + extensions: {}, + }, + }, ]; for (const tc of testcases) { diff --git a/src/common/options.ts b/src/common/options.ts index 229fc1f27..6f9fd650c 100644 --- a/src/common/options.ts +++ b/src/common/options.ts @@ -19,12 +19,27 @@ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +const RESET_VALUE_TAG = Symbol.for("firebase-functions:ResetValue:Tag"); + /** * Special configuration type to reset configuration to platform default. * * @alpha */ export class ResetValue { + /** + * Handle the "Dual-Package Hazard". + * + * We implement custom `Symbol.hasInstance` to so CJS/ESM ResetValue instances + * are recognized as the same type. + */ + static [Symbol.hasInstance](instance: unknown): boolean { + return (instance as { [RESET_VALUE_TAG]?: boolean })?.[RESET_VALUE_TAG] === true; + } + + get [RESET_VALUE_TAG](): boolean { + return true; + } toJSON(): null { return null; } diff --git a/src/params/index.ts b/src/params/index.ts index cde9ecf3c..baef1c760 100644 --- a/src/params/index.ts +++ b/src/params/index.ts @@ -46,7 +46,28 @@ export { Expression }; export type { ParamOptions }; type SecretOrExpr = Param | SecretParam | JsonSecretParam; -export const declaredParams: SecretOrExpr[] = []; + +/** + * Use a global singleton to manage the list of declared parameters. + * + * This ensures that parameters are shared between CJS and ESM builds, + * avoiding the "dual-package hazard" where the src/bin/firebase-functions.ts (CJS) sees + * an empty list while the user's code (ESM) populates a different list. + */ +const majorVersion = + // @ts-expect-error __FIREBASE_FUNCTIONS_MAJOR_VERSION__ is injected at build time + typeof __FIREBASE_FUNCTIONS_MAJOR_VERSION__ !== "undefined" + ? // @ts-expect-error __FIREBASE_FUNCTIONS_MAJOR_VERSION__ is injected at build time + __FIREBASE_FUNCTIONS_MAJOR_VERSION__ + : "0"; +const GLOBAL_SYMBOL = Symbol.for(`firebase-functions:params:declaredParams:v${majorVersion}`); +const globalSymbols = globalThis as unknown as Record; + +if (!globalSymbols[GLOBAL_SYMBOL]) { + globalSymbols[GLOBAL_SYMBOL] = []; +} + +export const declaredParams: SecretOrExpr[] = globalSymbols[GLOBAL_SYMBOL]; /** * Use a helper to manage the list such that parameters are uniquely diff --git a/src/params/types.ts b/src/params/types.ts index e937e2e33..14f7ce69d 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -22,12 +22,27 @@ import * as logger from "../logger"; +const EXPRESSION_TAG = Symbol.for("firebase-functions:Expression:Tag"); + /* * 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 * an Expression as the value of an option that normally accepts numbers. */ export abstract class Expression { + /** + * Handle the "Dual-Package Hazard" . + * + * We implement custom `Symbol.hasInstance` to so CJS/ESM Expression instances + * are recognized as the same type. + */ + static [Symbol.hasInstance](instance: unknown): boolean { + return (instance as { [EXPRESSION_TAG]?: boolean })?.[EXPRESSION_TAG] === true; + } + + get [EXPRESSION_TAG](): boolean { + return true; + } /** Returns the expression's runtime value, based on the CLI's resolution of parameters. */ value(): T { if (process.env.FUNCTIONS_CONTROL_API === "true") { diff --git a/tsdown.config.mts b/tsdown.config.mts index 3f40420bb..248f9bff4 100644 --- a/tsdown.config.mts +++ b/tsdown.config.mts @@ -1,4 +1,8 @@ import { defineConfig } from "tsdown"; +import { readFileSync } from "fs"; + +const pkg = JSON.parse(readFileSync("./package.json", "utf-8")); +const majorVersion = pkg.version.split(".")[0]; const rewriteProtoPathMjs = { name: "rewrite-proto-path-mjs", @@ -13,6 +17,11 @@ const rewriteProtoPathMjs = { // Note: We use tsc (via tsconfig.release.json) for .d.ts generation instead of tsdown's // built-in dts option due to issues with rolldown-plugin-dts. // See: https://github.com/sxzz/rolldown-plugin-dts/issues/121 + +const define = { + __FIREBASE_FUNCTIONS_MAJOR_VERSION__: JSON.stringify(majorVersion), +}; + export default defineConfig([ { entry: "src/**/*.ts", @@ -23,6 +32,7 @@ export default defineConfig([ dts: false, // Use tsc for type declarations treeshake: false, external: ["../../../protos/compiledFirestore"], + define, }, { entry: "src/**/*.ts", @@ -33,5 +43,6 @@ export default defineConfig([ dts: false, // Use tsc for type declarations treeshake: false, plugins: [rewriteProtoPathMjs], + define, }, ]); \ No newline at end of file