-
Notifications
You must be signed in to change notification settings - Fork 59
Incorrect "wrap" typing for v2 onCall cloud functions #163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@trex-quo Thank you for the bug report! V2 Callable Function wrapping has not been implemented yet. Theres going to need to be some work to implement this. I've added the "feature request" label and will follow up on this thread when we implement it. Thank you again!! |
Hello, I wanted to check in on this. I see that 2nd gen firestore triggers have been added, but was wondering if onCall wrapping has been implemented yet? Having been following the v2 function development closely, it seems like onCalls would be covered ahead of firestore triggers |
Gen2 functions are GA now. Being unable to write tests for them is a showstopper for us to migrate to gen2. @TheIronDev , any chance you could bump the priority on this one? |
I migrated all Functions to v2, then found wrap has not implemented yet. I think at least the documentation should warn the user NOT to upgrade to v2. @TheIronDev, Is there any timeline to release it? |
If it's helpful, it is still possible to use the existing library for wrapping on call functions. My team used some trial and error to find that you can invoke the wrapped function with its v2 arguments as so:
You can see that if using typescript, you may need to add a ts-ignore statement since the proper types don't exist yet. This does not work for firestore trigger functions, however. Those are entirely broken and untestable with this library. |
Howdy! @taeold would be the best person that could speak to timelines. Cheers, |
Thanks @trevor-rex! Since it's involved testing and the code quality, I prefer to use something that is "officially" supported. @taeold Do you know when will this be ready? |
@TheIronDev, who else would know the timeline besides @taeold ? |
The documentation should clearly tell users NOT to migrate to v2. Tests are a big part of a maintanable project, and if this is not ready, the documentation should let us know. |
Any progress on this? |
Happy 2024! And I would like to update my function to v2 as my resolution, if this issue is fixed. @TheIronDev @taeold Any update on this issue? |
Are firebase V2 functions really not properly testable, still after 2 years ? |
Looks like I am not the only one waiting for an answer for this long due question... |
@TheIronDev is there a timeline to make trigger functions and onCall testable? It seems weird to promote functions V2 when testing them is not full supported. |
Sorry, @taeold seems to be the right person to ask! So, any chance to see this in 2024? |
Any chance? |
Currently, @TheIronDev is the assignee. Should this issue be reassigned to @taeold or someone else to get a better attention to this issue? |
I still didn't migrate to v2 because of this. |
Unassigning @TheIronDev as he no longer works on the product. Will put this on our internal tracker to prioritize |
@inlined it's almost another month now. Do you have at least some ideas when will this be implemented? Encouraging people to move to v2, yet not supporting wrap doesn't make sense. |
We ran into this issue and were able to solve it with the following generic interface as a temporary workaround. Add this helper definition and use it when you're wrapping V2 callable functions. /**
* CallableV2Request is a helper interface to give typings to v2 cloud functions
* when testing using firebase-functions-test. It is a stripped down version of
* CallableRequest from firebase-functions/lib/common/providers/https.
*
* Example usage (in spec file):
* import fftest from "firebase-functions-test";
* const fft = fftest(*firebase config here*);
* const wrappedFunction = fft.wrap<
* CallableV2Request<CallableFunctionDataInterface>
* >(actualFunction as any);
* */
export interface CallableV2Request<T = any> {
data: T;
auth?: {
uid: string;
};
} Hope this helps while they finish updating this library. |
Thanks @jackarens! Do you aware of any limitation or issue with this approach? @inlined will you recommend this as a fair workaround before the real solution is in place? |
Seconding @cupidchan on thanking @jackarens for the workaround. Worked perfectly for me for onCall function wrapping. I will note that while v2 triggers did not give a type error anymore, I still had to refactor all my tests to replace calls to test.makeObjectMetadata() and test.makeDocumentSnapshot() with mocked StorageEvents and FirestoreEvents. Hoping this gets bumped up in priority. The main developer documentation currently recommends switching over to gen2 but if you dig into the API reference, most interfaces are marked as beta, which makes complete firebase-functions-test support for v2 critical for adoption. |
@inlined have you got a chance to review the workaround above? Also, do you know when will this be officially support? This issue has been opened for a while, it will be great if we have a timeline. Will that be you or someone else working on this? |
I don't think merely coercing the type definitions will work here, because the library actually fills in dummy values. We've gotten some more staffing for our various functions SDKs and I can put this third on our priority list (behind a new event type and a scheduled functions emulator). In the meantime, you can just call |
Thanks @inlined do you have any rough timeline for this to be done, maybe by Google Next 2024 next month? 😄 BTW. I will be there in case you want to meet and greet! |
Since all cloud functions have a Sadly, I won't be at GCP Next, though Firebase will have a presence there (including the GPM who oversees Functions). If you happen to be coming to I/O as well, I'll likely be there and even if I'm not can drive down to grab happy hour with our devs. |
fyi, for anyone that's having trouble getting the work-around types to work properly - I ended up using this import firebaseFunctionsTest from 'firebase-functions-test';
import { FeaturesList } from 'firebase-functions-test/lib/features';
...
type V2CallableReturn<T> = ReturnType<typeof onCall<T>>;
type WrapV2Function = <T>(
cloudFunction: V2CallableReturn<T>
) => ReturnType<FeaturesList['wrap']>;
export type WrapV2Features = Omit<FeaturesList, 'wrap'> & {
wrapV2: WrapV2Function;
};
export function _initializeFunctionsTest(): WrapV2Features {
const { wrap, ...features } = firebaseFunctionsTest(
// <your config here>
);
function wrapV2<T>(cloudFunction: V2CallableReturn<T>) {
return wrap(cloudFunction as any);
}
return {
...features,
wrapV2,
};
} Then elsewhere just call the Let me know if I missed anything! |
@SupposedlySam, how is this different from the one suggested by @jackarens, which looks much simpler?
|
Good question @cupidchan. The main difference is that I'm pulling the type directly from the |
Thanks @SupposedlySam! It looks like it can be the "fix" than a workaround. @inlined, do you think you can adopt this approach in your solution?
|
@exaby73 when do you think the PR will be in, as it's been a month already... |
@cupidchan Hey. Sorry I've been preoccupied with some other tasks. I am planning on completing this this week :) It ready, just some typing that needs to be fixed |
That's wonderful @exaby73 This long waiting feature is finally here!! Looking forward to the final merge of your PR! |
Any news on this? @cupidchan |
@sebastianrueckerai, this question should actually address to @exaby73. I also got an email saying v2 will become the default version soon so we really need this fixed in PROD. |
Why is this issue closed? I haven't seen a solution yet? |
@tpiaggio, I have just upgrade to the latest firebase and the wrapper is available for the v2 functions. I also upgrade my V1 to V2 along with tests. Please try and see if that works for you too. |
Yeah, I managed to make it work, thanks @cupidchan. The key for me was to use |
We are almost there to have a fully working support for v2. There is still an issue with callables ( const wrapped = wrap(myCallable);
await wrapped({
data: {
'foo': 'bar',
},
} as any); // here because of the following compilation error:
For sure as a testing library, we don't wanna pass the rawRequest as param (which is mandatory for now in the typings). |
Version info
firebase-functions-test: 2.3.0
firebase-functions: 3.22.0
firebase-admin: 9.1.1
Test case
Live code:
Note the above syntax differences from v1 onCall functions. There is no longer a pair of
(data, context)
parameters, only a singlefunctions.https.CallableRequest<any>
parameter. Additionally, the function options (e.g.minInstances
) is a parameter instead of a sub-function of thefunctions
class.Test code:
Steps to reproduce
Run the above sample with a
//@ts-ignore
above theawait wrappedFunction(...)
line. It works fine and seems like the wrapping for the new v2 functions works well. However, when removing the//@ts-ignore
line, I get a type error. If I try to run the test code with the old v1 syntax(data, context)
it does not work either.Expected behavior
No type error when running the above code snippet, as the code is correct. v2 function overload signature needs to be added.
Actual behavior
Receive type error
The text was updated successfully, but these errors were encountered: