search

firebase / firebase-functions-test

MIT License
232 stars 48 forks source link

Use conditional typing for defining the right set of options #166

Closed phcoliveira closed 1 year ago

phcoliveira commented 1 year ago

Description

This PR fixes the typing of a wrapped function's second argument, namely: options. Such options are described by two types: CallableContextOptions and EventContextOptions. The first is only due to the http.onCall(), since the function http.onRequest() is not supposed to be wrapped. The latter is due to all other cloud functions under V1.

With the current implementation, the type of the options is the result of a union of both types mentioned above. Because of that, a consumer of this API was led to believe that some options due to a "event" function were available to "callable" function, and vice-versa.

This PR fixes that by conditionally assigning the type of such options depending on the function being wrapped.

Code sample

There is no API change. But here is an example of what this PR introduces:

import * as functions from 'firebase-functions'
import firebaseFunctionsTest from 'firebase-functions-test'

const testFunctions = firebaseFunctionsTest()

const httpsOnCall = functions.https.onCall(/* ... */)
const wrappedHttpsOnCall = testFunctions.wrap(httpsOnCall, { /* CallableContextOptions */ })

const databaseOnCreate = functions.database.onCreate(/* ... */)
const wrappedDatabaseOnCreate = testFunctions.wrap(databaseOnCreate, { /* EventContextOptions */ })
google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

TheIronDev commented 1 year ago

@phcoliveira Thanks for opening up a pull request! The code makes sense to me.

I triggered the workflow actions, but it looks like you still need to sign the CLA.

phcoliveira commented 1 year ago

@phcoliveira Thanks for opening up a pull request! The code makes sense to me.

I triggered the workflow actions, but it looks like you still need to sign the CLA.

Good to hear, @TheIronDev. I will check this CLA thing in the morning. Bedtime for me.

phcoliveira commented 1 year ago

I signed the CLA, @TheIronDev.

TheIronDev commented 1 year ago

@phcoliveira

This is definitely an improvement!! Thank you very much for the pull request!

We don't cut new releases on Fridays, but I'll follow up with publishing a patch on Tuesday! (the next business day)

Thank you again! Tyler

phcoliveira commented 1 year ago

Happy to help, @TheIronDev. Closes #162