firebase-functions icon indicating copy to clipboard operation
firebase-functions copied to clipboard

Typescript: functions.https.onRequest() res mismatched type with cors middleware res

Open mikgross opened this issue 4 years ago • 8 comments

Related issues

[REQUIRED] Version info

node:

10

firebase-functions: 3.7 firebase-tools: 7.5 firebase-admin: 8.6

[REQUIRED] Test case

const corsCheck = cors({
    origin : [
        // dev url
        'http://localhost:4200'
    ],
    methods: 'POST',
    credentials: true
})

export const userSignIn = functions.https.onRequest((req, res: functions.Response) => {

    corsCheck(req, res, async () => {
        res.status(200).send('Nice');
    }
}

[REQUIRED] Steps to reproduce

run deploy command firebase deploy --only functions

[REQUIRED] Expected behavior

Correctly deploys my function

[REQUIRED] Actual behavior

The deploy fails, because of type Repsonse from functions doesn't match with type Response expected by CORS: Argument of type 'Response' is not assignable to parameter of type 'Response'. Type 'Response' is missing the following properties from type 'Response': status, sendStatus, links, send, and 70 more.

Were you able to successfully deploy your functions?

error TS2345: Argument of type 'Response' is not assignable to parameter of type 'Response'.

mikgross avatar Jun 22 '20 12:06 mikgross

@mikgross I am seeing similar issues since upgrading my firebase-functions version.

Did you manage to resolve this?

ralphcode avatar Aug 30 '20 08:08 ralphcode

similar issue here

phongyewtong avatar Nov 04 '20 05:11 phongyewtong

Hey there, this is an issue we'll need to look at. functions.http.Request is actually an express.Request, but we redefined what it was to add on some additional type info (e.g. that body was now JSON due to bodyparser). We can look into fixing this (though there might be some reverse compatibility concerns). For now, though, if you forcibly cast it to express.Request it should work.

    corsCheck(req as any as express.Request, res as any as express.Response, async () => {
        res.status(200).send('Nice');
    }

inlined avatar May 26 '21 17:05 inlined

Thanks for the workaround. Will definitly try it!

mikgross avatar Jun 04 '21 04:06 mikgross

I dove into the code and think I may see the bug. We have

export interface Request extends express.Request {
  rawBody: Buffer;
});

which seems right. Except that express.Request is a generic interface. By extending Request we're extending a type with fixed defaults, which may be causing the issues you see. I'll dive in more when I have a chance.

inlined avatar Aug 30 '21 18:08 inlined

Believe we have the same issue, is this still a known problem?

R-Iqbal avatar Nov 23 '21 13:11 R-Iqbal

Yes. It's a low priority since a cast does the trick. Internal bug #209905562

inlined avatar Dec 09 '21 04:12 inlined

Yes. It's a low priority since a cast does the trick. Internal bug #209905562

But also super annoying for someone trying to figure out why types are all over the place until finally landing on this thread.

Not only that you're bypassing type safety by doing so .. especially if something is changed firebase later on (which seems to be common with Google)

... no wonder all the examples are in javascript and not typescript

To completely bypass type safety as suggested above:

 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
onRequest(async (request: any, response: any) => {
  request = request as Request;
  response = response as Response;
...
}

tripflex avatar Aug 15 '23 17:08 tripflex