Skip to content

Add firebase-functions v7 CI leg; make mockConfig() throw on v7+ - #334

Open
cabljac wants to merge 5 commits into
masterfrom
test/firebase-functions-version-matrix
Open

Add firebase-functions v7 CI leg; make mockConfig() throw on v7+#334
cabljac wants to merge 5 commits into
masterfrom
test/firebase-functions-version-matrix

Conversation

@cabljac

@cabljac cabljac commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

Refs #333 - implements the first two parts of the plan discussed there (the latest canary job is a follow-up and is intentionally not in this PR, so this does not close the issue).

1. mockConfig() now throws on firebase-functions v7+ (behavior change)

functions.config() was removed in firebase-functions v7 (it throws at runtime and is typed never). Previously mockConfig() would silently "succeed" on a v7 install while every subsequent config() read threw - a confusing trap. mockConfig() now detects the installed firebase-functions major version and fails at the mock site with migration guidance:

mockConfig() is not supported with firebase-functions v7+ because functions.config() was removed. Migrate to environment parameters using the params module.

Detection resolves the installed firebase-functions entry point and walks up to its package.json to read the version (the package's exports map does not expose ./package.json, on v4 or v7, so a plain require('firebase-functions/package.json') does not work). If the version cannot be determined, it falls back to feature-detecting config() on the v1 entry point.

Behavior on v4-v6 installs is unchanged.

2. CI matrix: unit tests run against both pinned firebase-functions and v7

The unit job gains a firebase-functions: [pinned, '7'] matrix dimension. The pinned leg is exactly today's flow. The 7 leg compiles the specs against the pinned devDependency, swaps in firebase-functions@^7 with npm install --no-save, and runs mocha on the already-compiled output. Compile-then-swap is needed because the specs do not compile against v6/v7 typings (nested @types/express lib-check conflict, plus config typed never), and it mirrors how users actually hit version drift (#310): their compiled code running against a newer firebase-functions.

The #mockConfig specs are version-aware: on the pinned leg they assert the existing mocking behavior; on the v7 leg they assert the new error (and that CLOUD_RUNTIME_CONFIG is left untouched). The two lifecycle env-restore tests seed CLOUD_RUNTIME_CONFIG directly on v7+ since their subject is cleanup(), not mockConfig(). Nothing is skipped on either leg.

Both legs are meant to be required checks - they target pinned majors, not floating latest, so they cannot go red from an unrelated upstream release. A scheduled non-blocking canary against firebase-functions@latest is the remaining part of #333.

Verified locally on Node 22:

  • pinned (4.9.0): npm ci && npm test - 119 passing, 0 failing
  • v7 (7.2.5): npx tsc, npm install --no-save firebase-functions@^7, npx mocha .tmp/spec/index.spec.js - 119 passing, 0 failing (v7 mockConfig assertions confirmed running), lint clean

Code sample

// with firebase-functions@^7 installed:
const test = require('firebase-functions-test')();
test.mockConfig({ slack: { url: 'https://...' } });
// throws: mockConfig() is not supported with firebase-functions v7+ because
// functions.config() was removed. Migrate to environment parameters using
// the params module.

functions.config() was removed in firebase-functions v7, so mockConfig()
silently succeeding there is a trap: the mock "works" but every
subsequent config() read throws. mockConfig() now detects the installed
firebase-functions major version and throws with migration guidance on
v7+, which makes the >=4.9.0 peer range honest.

The unit CI job gains a firebase-functions matrix dimension: the
existing pinned leg, plus a v7 leg that compiles the specs against the
pinned typings, swaps in firebase-functions@^7, and runs mocha on the
compiled output (the specs do not compile against v7 typings). The
mockConfig specs are version-aware and assert the throw on v7.

Refs #333

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for detecting the major version of firebase-functions to handle the removal of functions.config() in v7+, causing mockConfig() to throw an error with migration guidance on newer versions. The feedback suggests making the version parsing more robust by using parseInt to prevent NaN results, and exporting a unified _isConfigRemoved() helper to ensure consistent detection logic across both the source code and the test suites.

Comment thread src/v1.ts
Comment thread src/v1.ts Outdated
Comment thread spec/lifecycle.spec.ts Outdated
Comment thread spec/main.spec.ts Outdated
Comment thread spec/main.spec.ts Outdated
cabljac added 3 commits July 23, 2026 10:12
- export _isConfigRemoved() and use it in both spec files instead of
  branching on _firebaseFunctionsMajorVersion()
- add defensive typeof check on config before probing
- parse major version with parseInt and reject NaN instead of
  returning it
- clear and restore K_CONFIGURATION around the feature-detect
  fallback so v4-v6 is not misclassified under GCFv2
- add CHANGELOG entry for the mockConfig() v7+ migration error
- run lint/format only on the pinned CI matrix legs
@cabljac

cabljac commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces changes to handle the removal of functions.config() in firebase-functions v7+. It adds detection logic to check the installed major version of firebase-functions and updates mockConfig() to throw a clear migration error when v7+ is detected. Test suites have been updated accordingly. The review feedback suggests caching the result of the _isConfigRemoved() check to avoid redundant file I/O and environment variable manipulation on subsequent calls.

Comment thread src/v1.ts
@cabljac
cabljac requested review from CorieW and IzaakGough July 23, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant