Skip to content

Commit 5bdf3eb

Browse files
authored
fix: Resolve dual-package hazard for params in ESM (#1780)
* fix: Resolve dual-package hazard for params in ESM We resolve two critical issues caused by the "Dual-Package Hazard" when using `firebase-functions` in ESM projects: 1. Separate instances of the `declaredParams` in `firebase-functions/params` led to params not being detected when used in ESM modules. To fix, we implement a Global Singleton pattern for `declaredParams` using `globalThis` and a version-scoped `Symbol.for` key. This ensures both builds share the same global storage for collecting param API uses. 1. `instanceof` checks failed because the CJS `Expression`/`ResetValue` instances deferred from the ESM ones. To fix, we implement a custom `[Symbol.hasInstance]` on `Expression` and `ResetValue` classes using specfic branded tags to identify instances across package boundaries. * fix: Resolve dual-package hazard for params in ESM This commit resolves two critical issues caused by the "Dual-Package Hazard" when using `firebase-functions` in ESM projects: 1. **Split State (Missing Params)**: The SDK bootstrap code (CJS) and User Code (ESM) were using separate instances of the `declaredParams` array. * **Fix**: Implemented a Global Singleton pattern for `declaredParams` using `globalThis` and a version-scoped `Symbol.for` key. This ensures both builds share the same storage. 2. **Identity Mismatch (Deployment Error)**: `instanceof Expression` checks in the SDK bootstrap code failed because the user's `Expression` instance (ESM) differed from the bootstrap's class (CJS). * **Fix**: Implemented `[Symbol.hasInstance]` on `Expression` and `ResetValue` classes. * **Mechanism**: Uses `Symbol.for` tags (`firebase-functions:Expression:Tag`) to robustly identify instances across package boundaries, avoiding fragile duck-typing or strict class identity checks. This ensures that parameterized configuration works correctly regardless of the module system used. * fix changelog * clarify comments. * formatter
1 parent 2c2c78d commit 5bdf3eb

File tree

10 files changed

+140
-1
lines changed

10 files changed

+140
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fix "Dual-Package Hazard" for parameterized configuration in ESM projects. (#1780)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const { defineInt } = require("firebase-functions/params");
2+
const { onRequest } = require("firebase-functions/v2/https");
3+
4+
const minInstances = defineInt("MIN_INSTANCES", { default: 1 });
5+
6+
exports.v2http = onRequest({ minInstances }, (req, res) => {
7+
res.send("PASS");
8+
});
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "commonjs-params",
3+
"main": "index.js"
4+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { defineInt } from "firebase-functions/params";
2+
import { onRequest } from "firebase-functions/v2/https";
3+
4+
const minInstances = defineInt("MIN_INSTANCES", { default: 1 });
5+
6+
export const v2http = onRequest({ minInstances }, (req, res) => {
7+
res.send("PASS");
8+
});
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "esm-params",
3+
"type": "module"
4+
}

scripts/bin-test/test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,32 @@ describe("functions.yaml", function () {
362362
extensions: BASE_EXTENSIONS,
363363
},
364364
},
365+
{
366+
name: "with params",
367+
modulePath: "./scripts/bin-test/sources/commonjs-params",
368+
expected: {
369+
endpoints: {
370+
v2http: {
371+
...DEFAULT_V2_OPTIONS,
372+
platform: "gcfv2",
373+
entryPoint: "v2http",
374+
labels: {},
375+
httpsTrigger: {},
376+
minInstances: "{{ params.MIN_INSTANCES }}",
377+
},
378+
},
379+
requiredAPIs: [],
380+
specVersion: "v1alpha1",
381+
params: [
382+
{
383+
name: "MIN_INSTANCES",
384+
type: "int",
385+
default: 1,
386+
},
387+
],
388+
extensions: {},
389+
},
390+
},
365391
];
366392

367393
for (const tc of testcases) {
@@ -396,6 +422,32 @@ describe("functions.yaml", function () {
396422
modulePath: "./scripts/bin-test/sources/esm-ext",
397423
expected: BASE_STACK,
398424
},
425+
{
426+
name: "with params",
427+
modulePath: "./scripts/bin-test/sources/esm-params",
428+
expected: {
429+
endpoints: {
430+
v2http: {
431+
...DEFAULT_V2_OPTIONS,
432+
platform: "gcfv2",
433+
entryPoint: "v2http",
434+
labels: {},
435+
httpsTrigger: {},
436+
minInstances: "{{ params.MIN_INSTANCES }}",
437+
},
438+
},
439+
requiredAPIs: [],
440+
specVersion: "v1alpha1",
441+
params: [
442+
{
443+
name: "MIN_INSTANCES",
444+
type: "int",
445+
default: 1,
446+
},
447+
],
448+
extensions: {},
449+
},
450+
},
399451
];
400452

401453
for (const tc of testcases) {

src/common/options.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,27 @@
1919
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
2020
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2121
// SOFTWARE.
22+
const RESET_VALUE_TAG = Symbol.for("firebase-functions:ResetValue:Tag");
23+
2224
/**
2325
* Special configuration type to reset configuration to platform default.
2426
*
2527
* @alpha
2628
*/
2729
export class ResetValue {
30+
/**
31+
* Handle the "Dual-Package Hazard".
32+
*
33+
* We implement custom `Symbol.hasInstance` to so CJS/ESM ResetValue instances
34+
* are recognized as the same type.
35+
*/
36+
static [Symbol.hasInstance](instance: unknown): boolean {
37+
return (instance as { [RESET_VALUE_TAG]?: boolean })?.[RESET_VALUE_TAG] === true;
38+
}
39+
40+
get [RESET_VALUE_TAG](): boolean {
41+
return true;
42+
}
2843
toJSON(): null {
2944
return null;
3045
}

src/params/index.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,28 @@ export { Expression };
4646
export type { ParamOptions };
4747

4848
type SecretOrExpr = Param<any> | SecretParam | JsonSecretParam<any>;
49-
export const declaredParams: SecretOrExpr[] = [];
49+
50+
/**
51+
* Use a global singleton to manage the list of declared parameters.
52+
*
53+
* This ensures that parameters are shared between CJS and ESM builds,
54+
* avoiding the "dual-package hazard" where the src/bin/firebase-functions.ts (CJS) sees
55+
* an empty list while the user's code (ESM) populates a different list.
56+
*/
57+
const majorVersion =
58+
// @ts-expect-error __FIREBASE_FUNCTIONS_MAJOR_VERSION__ is injected at build time
59+
typeof __FIREBASE_FUNCTIONS_MAJOR_VERSION__ !== "undefined"
60+
? // @ts-expect-error __FIREBASE_FUNCTIONS_MAJOR_VERSION__ is injected at build time
61+
__FIREBASE_FUNCTIONS_MAJOR_VERSION__
62+
: "0";
63+
const GLOBAL_SYMBOL = Symbol.for(`firebase-functions:params:declaredParams:v${majorVersion}`);
64+
const globalSymbols = globalThis as unknown as Record<symbol, SecretOrExpr[]>;
65+
66+
if (!globalSymbols[GLOBAL_SYMBOL]) {
67+
globalSymbols[GLOBAL_SYMBOL] = [];
68+
}
69+
70+
export const declaredParams: SecretOrExpr[] = globalSymbols[GLOBAL_SYMBOL];
5071

5172
/**
5273
* Use a helper to manage the list such that parameters are uniquely

src/params/types.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,27 @@
2222

2323
import * as logger from "../logger";
2424

25+
const EXPRESSION_TAG = Symbol.for("firebase-functions:Expression:Tag");
26+
2527
/*
2628
* A CEL expression which can be evaluated during function deployment, and
2729
* resolved to a value of the generic type parameter: i.e, you can pass
2830
* an Expression<number> as the value of an option that normally accepts numbers.
2931
*/
3032
export abstract class Expression<T extends string | number | boolean | string[]> {
33+
/**
34+
* Handle the "Dual-Package Hazard" .
35+
*
36+
* We implement custom `Symbol.hasInstance` to so CJS/ESM Expression instances
37+
* are recognized as the same type.
38+
*/
39+
static [Symbol.hasInstance](instance: unknown): boolean {
40+
return (instance as { [EXPRESSION_TAG]?: boolean })?.[EXPRESSION_TAG] === true;
41+
}
42+
43+
get [EXPRESSION_TAG](): boolean {
44+
return true;
45+
}
3146
/** Returns the expression's runtime value, based on the CLI's resolution of parameters. */
3247
value(): T {
3348
if (process.env.FUNCTIONS_CONTROL_API === "true") {

tsdown.config.mts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
import { defineConfig } from "tsdown";
2+
import { readFileSync } from "fs";
3+
4+
const pkg = JSON.parse(readFileSync("./package.json", "utf-8"));
5+
const majorVersion = pkg.version.split(".")[0];
26

37
const rewriteProtoPathMjs = {
48
name: "rewrite-proto-path-mjs",
@@ -13,6 +17,11 @@ const rewriteProtoPathMjs = {
1317
// Note: We use tsc (via tsconfig.release.json) for .d.ts generation instead of tsdown's
1418
// built-in dts option due to issues with rolldown-plugin-dts.
1519
// See: https://github.com/sxzz/rolldown-plugin-dts/issues/121
20+
21+
const define = {
22+
__FIREBASE_FUNCTIONS_MAJOR_VERSION__: JSON.stringify(majorVersion),
23+
};
24+
1625
export default defineConfig([
1726
{
1827
entry: "src/**/*.ts",
@@ -23,6 +32,7 @@ export default defineConfig([
2332
dts: false, // Use tsc for type declarations
2433
treeshake: false,
2534
external: ["../../../protos/compiledFirestore"],
35+
define,
2636
},
2737
{
2838
entry: "src/**/*.ts",
@@ -33,5 +43,6 @@ export default defineConfig([
3343
dts: false, // Use tsc for type declarations
3444
treeshake: false,
3545
plugins: [rewriteProtoPathMjs],
46+
define,
3647
},
3748
]);

0 commit comments

Comments
 (0)