Skip to content

Add generics to httpsCallable #4466

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

Merged
merged 7 commits into from
Feb 23, 2021
Merged

Add generics to httpsCallable #4466

merged 7 commits into from
Feb 23, 2021

Conversation

hsubox76
Copy link
Contributor

Add RequestData and ResponseData generics to httpsCallable. RequestData is the arg provided to the callable function (only one allowed). The callable function returns a promise with a data field, and RequestData describes whatever is in that data field.

Used the generics in a couple of httpsCallable tests as a check to make sure the typing works, and left them out of some tests to make sure it also works if they are omitted.

@changeset-bot
Copy link

changeset-bot bot commented Feb 12, 2021

⚠️ No Changeset found

Latest commit: 14dbd51

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 12, 2021

Binary Size Report

Affected SDKs

No changes between base commit (1003b8d) and head commit (3faaf66).

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 12, 2021

Size Analysis Report

Affected Products

No changes between base commit (1003b8d) and head commit (3faaf66).

@Feiyang1 Feiyang1 assigned hsubox76 and unassigned Feiyang1 Feb 17, 2021
@hsubox76 hsubox76 assigned Feiyang1 and unassigned hsubox76 Feb 17, 2021
@hsubox76 hsubox76 force-pushed the ch-httpscallable-type branch from fad56ab to d7692c2 Compare February 17, 2021 19:55
@@ -98,7 +101,7 @@ describe('Firebase Functions > Call', () => {

it('scalars', async () => {
const functions = createTestService(app, region);
const func = httpsCallable(functions, 'scalarTest');
const func = httpsCallable<number, number>(functions, 'scalarTest');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe at least the request (and maybe the response as well) must be objects. Scalars may not be allowed by the server-side implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a live test that sends a number to the server and successfully gets a number response so it seems to work, unless I'm misunderstanding. To make sure, I also created a new onCall function that logged a number sent to it and returned a number and tried it from a test app and it seems to work:

exports.numberTest = functions.https.onCall((data) => {
    console.log(data, typeof data);
    return(44);
});

Let me know if I misunderstood.

@hsubox76 hsubox76 force-pushed the ch-httpscallable-type branch from 14dbd51 to 4b7a353 Compare February 23, 2021 19:34
@hsubox76 hsubox76 force-pushed the ch-httpscallable-type branch from 4b7a353 to b4c1b5d Compare February 23, 2021 19:41
@hsubox76 hsubox76 merged commit 04b4986 into master Feb 23, 2021
@hsubox76 hsubox76 deleted the ch-httpscallable-type branch February 23, 2021 20:15
@firebase firebase locked and limited conversation to collaborators Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants