Added support to v2 onSchedule#282
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for wrapping Firebase V2 scheduled functions (ScheduleFunction) in the test wrapper library. It extends the existing wrapping functionality to handle scheduler.onSchedule() functions alongside the already-supported CloudFunction and CallableFunction types.
Key changes:
- Added ScheduleFunction support with type guards and wrapper implementation
- Added input validation for scheduled event options
- Enhanced null checking for cloud functions before wrapping
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/v2.ts | Added ScheduleFunction type support with type guards, wrapper creation, and option validation |
| src/main.ts | Enhanced type checking with null guard and updated type annotations |
| spec/v2.spec.ts | Added test coverage for scheduler.onSchedule() wrapping functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function assertIsCloudFunctionV2<T extends CloudEvent<unknown>>( | ||
| cf: CloudFunction<T> | CallableFunction<any, any> | ||
| ): asserts cf is CloudFunction<T> { | ||
| if (cf?.__endpoint?.platform !== 'gcfv2') { | ||
| throw new Error('This function can only wrap V2 CloudFunctions.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
The assertIsCloudFunctionV2 function should accept ScheduleFunction in its parameter type since it's called with a V2WrappableFunctions union type (line 117) which includes ScheduleFunction. Without this, TypeScript will raise a type error when ScheduleFunction is passed to assertIsCloudFunctionV2.
|
Hi @alexandercerutti. Thanks for your contribution! I'll be sure to review as soon as possible. |
|
@CorieW any news about this? |
Description
Fixes #210
I've added the tests to check if the things are working correctly and all the tests run perfectly. Same as linting.
The only catch, with the current changes, are that in order to test a scheduled function, since Firebase sets the provided handler to
onScheduleas therunfunction, the function that is provided insideonSchedulemust have at least one parameter, even if unused. This is not a thing that I really like, but we'd need to change howonScheduleactually works - and I'm not here for this.Another thing is that I had to change the implementation of
isV2CloudFunctionand remove one of the conditions. I don't like this either, but the tests are ending correctly.Imho, in order to detect the correct type of function,
wrapV2module should expose theassertIsCloudFunctionV2and we should use it... or convert the package to not have a middle switch point between v1 and v2 but have twoexports, one for v1 and one for v2.Let me know how does this feel.
Thank you!