Skip to content

Typescript error with V2 wrap: Type '{ data: WhatEverType; }' is missing the following properties from type 'CallableRequest<any>': rawRequest, acceptsStreaming #257

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

Open
sceee opened this issue Jan 31, 2025 · 8 comments
Labels
type: bug Something isn't working

Comments

@sceee
Copy link
Contributor

sceee commented Jan 31, 2025

Following up on the now closed #163 , there is still a type issue with wrap when trying to test v2 functions.

For example, if we just want to call the function:

await wrap(fnsmyV2Function)({ data: { whatever: "mydatais" } })

...this creates the following Typescript error:

Typescript error with V2 `wrap` Type '{ data: WhatEverTypeMyParameterTypeIs; }' is missing the following properties from type 'CallableRequest<any>': rawRequest, acceptsStreaming

It is indeed a usecase to set rawRequest sometimes in the wrapped request, e.g. if you want to fake some parameters of the request.

So I think that the wrapped function parameter should be typed Partial<CallableRequest<any>>.

Ideally, it should infer the request object any from the original CallableFunction definition so it is typed Partial<CallableRequest<WhatEverTypeMyParameterTypeIs>>.

Even better, the then optional rawRequest property should also be a Partial<Request> so we can only provide the values we need for a test.

@CorieW
Copy link

CorieW commented Feb 10, 2025

Hi @sceee,

Thanks for reporting this issue! We’ve received it and are reviewing it. We’ll provide updates as soon as possible.

@CorieW
Copy link

CorieW commented Feb 10, 2025

Have you tried using the wrapV2 function?

@CorieW CorieW added the Needs: Author Feedback Issues awaiting author feedback label Feb 10, 2025
@sceee
Copy link
Contributor Author

sceee commented Feb 10, 2025

Hi @CorieW ,

No, I used wrap as described here in the samples:
https://github.com/firebase/functions-samples/blob/0dcd3371ab553c4e44590e7e63c9f57f6f1af6f3/Node/test-functions-jest-ts/functions/src/__test__/index.test.ts#L24-L39

This sample is referenced on the Readme of this repo:
https://github.com/firebase/firebase-functions-test/blob/53f3e3e733e824835123f54bed75fde683d2c42e/README.md#examples

If these samples are outdated and instead wrapV2 should be used, the docs/samples need an update.

However, even if I try it with the wrapV2 function like this

const response = await wrapV2(myFunctions.fnsmyV2Function)({ data: { whatever: "mydatais" })

...I still get the same error:

Argument of type '{ data: WhateverTypeParameterMyDataIs; }' is not assignable to parameter of type 'CallableRequest<any>'.
  Type '{ data: WhateverTypeParameterMyDataIs; }' is missing the following properties from type 'CallableRequest<any>': rawRequest, acceptsStreamingts(2345)

@sbuys
Copy link

sbuys commented Feb 14, 2025

The function returned by wrap (automatically wrapV2 when used with a v2 function) is the callable request handler that you pass as the second parameter to onCall(opts, handler) ➡️

async (req: CallableRequest) => {
}

So we have to pass that entire request payload:

export interface CallableRequest<T = any> {
    data: T;
    app?: AppCheckData;
    auth?: AuthData;
    instanceIdToken?: string;
    rawRequest: Request;
    acceptsStreaming: boolean;
}

Something like this works, though there should be a better way to handle AppCheck and Auth than manually mocking:

const callableRequest: CallableRequest = {
      data: {},
      app: {
        appId: 'test-app',
        token: {
          aud: ['test-project-number', 'test-project-id'],
          exp,
          iat,
          iss: 'https://firebaseappcheck.googleapis.com/test-app',
          sub: 'test-app',
          app_id: 'test-app-id',
        },
      },
      auth: {
        uid: 'uid123',
        token: {
          email: '[email protected]',
          aud: 'test-project-id',
          auth_time: iat,
          exp,
          iat,
          firebase: {
            identities: {}, // provider-specific identity details
            sign_in_provider: 'custom', // provider used to sign in (e.g., "anonymous", "password")
            tenant: 'tenant-id', // tenant ID if applicable
          },
          iss: 'https://securetoken.google.com/test-project-id',
          sub: 'uid123', // uid
          uid: 'uid123',
        },
      },
      rawRequest: {} as unknown as CallableRequest['rawRequest'],
      acceptsStreaming: false,
}

@sceee
Copy link
Contributor Author

sceee commented Feb 16, 2025

Thanks @sbuys for your sample.

Of course, you can "make Typescript happy" using something like your mentioned rawRequest: {} as unknown as CallableRequest['rawRequest'], but that's definitely not a good way as it just overwrites the type of {} to look like a CallableRequest['rawRequest'] regardless of its actual type.

But during runtime, {} is still {} instead of being a valid request object so it could break at any time if some code depends on something in the request object.

So instead of having to just fake the types to make TS happy, I would prefer if firebase-functions-test handles this for both affected properties rawRequest and acceptsStreaming (and even if it's just faking the types for now or providing a mocked request object - at least it's then under control of the testing library).

The need that users of this library would have to do this does not seem logical to me.

As I wrote in the description of this issue, I would prefer if firebase-functions-test would support providing parts of the rawRequest optionally in wrapV2 in case you need to overwrite something that is used in your function under test.

@sbuys
Copy link

sbuys commented Feb 16, 2025

Thanks @sbuys for your sample.

Of course, you can "make Typescript happy" using something like your mentioned rawRequest: {} as unknown as CallableRequest['rawRequest'], but that's definitely not a good way as it just overwrites the type of {} to look like a CallableRequest['rawRequest'] regardless of its actual type.

But during runtime, {} is still {} instead of being a valid request object so it could break at any time if some code depends on something in the request object.

So instead of having to just fake the types to make TS happy, I would prefer if firebase-functions-test handles this for both affected properties rawRequest and acceptsStreaming (and even if it's just faking the types for now or providing a mocked request object - at least it's then under control of the testing library).

The need that users of this library would have to do this does not seem logical to me.

As I wrote in the description of this issue, I would prefer if firebase-functions-test would support providing parts of the rawRequest optionally in wrapV2 in case you need to overwrite something that is used in your function under test.

Oh I agree with you - in its current form, wrap doesn't appear to do much at all and simply passes through the full CallableRequest argument. If you don't want to bail out and cast {} as rawRequest, you could use a library like @jest-mock/express to create the Request object since the firebase raw request extends express request.

For the testing library to be useful, it should really help with the boilerplate required to build up CallableRequest like it kinda did with eventContextOptions in v1.

Testing has always felt like an afterthought with firebase though. The tooling / documentation has always been weak. Right now we find it easier to just run everything "offline" and call our functions directly, mocking all firebase calls. This requires a good amount of work to setup initially but it means we aren't reliant on testing tools from firebase, which historically have not been a priority. Also depending on the emulators is risky business.

@sceee
Copy link
Contributor Author

sceee commented Feb 19, 2025

@CorieW just as reminder as I feel something is wrong with the "Needs author feedback" label:
I don't know why this issue still has this label as I've given feedback already and the label probably should have been removed automatically.

@CorieW
Copy link

CorieW commented Feb 19, 2025

@sceee Yeah, you're right, it should be removed automatically. Though, not to worry, I'm aware you've given feedback. I'm just a bit tied up with other work at the moment, but I'll see about investigating this issue as soon as possible. Thanks for your patience!

@CorieW CorieW added type: bug Something isn't working and removed Needs: Author Feedback Issues awaiting author feedback labels Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants