feature: functions can now declare lifecycle hooks - #1915
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for post-deployment lifecycle hooks (afterInstall and afterUpdate) by adding a new lifecycle module, updating the manifest stack loader to capture these hooks, and exposing the module exports in package.json. The review feedback highlights a correctness issue in src/runtime/loader.ts where falsy but valid JSON values (such as 0 or false) are incorrectly omitted during serialization, and suggests using TypeScript union types in src/lifecycle/index.ts to enforce mutually exclusive properties at compile-time.
02d17b8 to
9489ef4
Compare
…e falsy body/params in loader
inlined
left a comment
There was a problem hiding this comment.
Don't forget that when we publish this it should also go in index.ts
You're going to need to add the fake empty file to avoid eslint problems (v2 not src/v2)
inlined
left a comment
There was a problem hiding this comment.
Sorry, forgot to set approve. I trust that this feedback is straightforward enough to fix if you also agree about the expression thing
* Clean up use of Expression<String> * Directly load from the manifest for hooks * Clean up HttpAction def * Add lifecycle to index.ts * Add empty index.js file for eslint
ajperel
left a comment
There was a problem hiding this comment.
Some mostly nitty comments after the fact as I review this PR to learn more than find issues. But still could be nice to address eventually. I could potentially send a PR myself if you prefer?
| | { call: CallAction; task?: never; http?: never } | ||
| | { http: HttpAction; task?: never; call?: never }; | ||
|
|
||
| const majorVersion = |
There was a problem hiding this comment.
I was definitely like "Why is this here and why do we need it?" I'd suggest having a nice comment like params/index.ts does to explain what's going on here.
| params?: Record<string, unknown>; | ||
| } | ||
|
|
||
| export type HttpAction = ({ function: string } | { url: string }) & { |
There was a problem hiding this comment.
Does the URL here need to accept string | Expression<string> similar to ManifestLifecycleAction below to fully support CEL expressions?
There was a problem hiding this comment.
I missed updating ManifestLifecycleAction to only accept a string. None of these fields should be parameterized. I'll update this in a follow-up PR along with the documentation comment above.
| if (Object.keys(declaredLifecycleHooks).length > 0) { | ||
| stack.lifecycleHooks = { ...declaredLifecycleHooks }; | ||
| } | ||
| clearDeclaredLifecycleHooks(); |
There was a problem hiding this comment.
clearDeclaredLifecycleHooks() documentation said it was primarily for testing. Why do you need to call it here?
Seems like you do need it here similar to clearGlobalRequiredAPIs(); and maybe we should have a slightly different comment on the method itself. IF AI is correct because loadStack() will be called multiple times when reloading things for the emulator?
There was a problem hiding this comment.
Hm, I can see how the "primarily for testing" bit can cause confusion since it's also used to clear the cache to avoid throwing errors about the hooks being already defined on subsequent loads. I'll remove that piece of the docstring in clearDeclaredLifecycleHooks in the follow-up PR.
…1929) # Description Fixes Cloud Build substitution errors when running prerelease/force deploys. Also had to change a GitHub repository setting to make sure the commit message is the PR description. The previous relnotes were lost and are added back here. # Release Notes relnote: fix: Remove false warning when using Expression in cors option (#1802) relnote: feat: Add requiresRole developer API for declarative security support and automatic Manifest extraction (#1908) relnote: Validate literal `timeoutSeconds` values per v2 trigger type (0-540s for events, 0-3600s for HTTPS/callable, 0-1800s for task queues, 0-7s for identity functions) so misconfigured values fail at function-definition or manifest-extraction time instead of at deploy time. (#1877) relnote: feat: Add requiresAPI function to allow declaring Google Cloud API dependencies in code. (#1900) relnote: fix(v1): Call onInit for schedule.onRun functions (#1801) relnote: feat: Add support to declare lifecycle hooks in functions. (#1915) relnote: fix(cors): Fix issue using Params to set CORS allowed hosts (#1903) relnote: fix(v2): Fix event data unpacking for auth event triggers (#1923) relnote: feat: Add "v2/lifecycle" and "lifecycle" import paths for lifecycle hooks (#1926) relnote: chore: revamp deploy pipeline to be stateless. Changes must now include relnotes relnote: chore: move the last encrypted keys into Google Cloud Secrets Manager
Description
This PR adds support for declaring codebase lifecycle hooks in the Firebase Functions SDK. Developers can now register actions (
afterInstallandafterUpdate) that are automatically executed post-deployment.Summary of Changes
1. Hook Registrations (
src/lifecycle/index.ts)firebase-functions/lifecycle:afterInstall(action: LifecycleAction): Registers a hook to run when resources in the codebase are deployed for the first time.afterUpdate(action: LifecycleAction): Registers a hook to run when resources in the codebase are updated.task: Triggers a task queue function with an optional payloadbody.call: Calls a function (callable trigger) with optionalparams.http: Triggers a HTTP endpoint via requesturl/function,method, andbody.afterInstallandafterUpdatecan be registered per codebase at runtime.2. Stack Manifest Parsing (
src/runtime/loader.ts,src/runtime/manifest.ts)ManifestLifecycleActionand updatedManifestStacktypes to include thelifecycleHooksfield.loadStackinsideloader.tsto capture any declared lifecycle hooks from global symbols during function discovery, mapping them into the compiled JSON/wire format returned tofirebase-tools.stackToWireserialization helper to recursively resolve CEL expressions and reset values insidelifecycleHooksproperties.3. Tests
spec/lifecycle/lifecycle.spec.tsto test hook registration and duplicate hook prevention.spec/runtime/loader.spec.tsverifying thatafterInstallandafterUpdatedeclarations are correctly discovered and serialized into theManifestStack.Code sample