Skip to content
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

Improve types for Runnable #995

Closed
wants to merge 2 commits into from
Closed

Improve types for Runnable #995

wants to merge 2 commits into from

Conversation

danprince
Copy link

@danprince danprince commented Oct 17, 2021

Swaps Runnable<T> for Runnable<Data, Return> to preserve inferred types when using the Runnable interface (or it's derivative CloudFunction).

This helps with writing tests that call functions, because their return type will be preserved.

import * as functions from "firebase-functions";

const double = https.onCall((n: number) => n * 2);
const four: number = await double.run(2, {});

However, the main motivation for this change is to help with writing type-safe client/server interactions. The runnable interface gives the client a chance to infer the function's signature.

Functions

functions/index.ts
import * as functions from "firebase-functions";

// double is Runnable<number, number>
export const double = functions.https.onCall((n: number) => n * 2);

Client

client/index.ts
import { getFunctions } from "firebase/functions";
import { httpsCallables } from "./https-callables";

// ...
let functions = getFunctions(app);
let callable = httpsCallables<typeof import("../functions")>(functions);

let double = callable("double");

let result: string = await double("hello");
//          ^^^^^^                ^^^^^^^
// Compiler knows that return type and parameter are wrong!
client/https-callables.ts
import { Functions, httpsCallable } from "firebase/functions";

interface Runnable<Data = any, Return = any> {
  run(data: Data): Return | PromiseLike<Return>;
}

type GetCallableFunctions<ImportedFunctions> = {
  [Name in keyof ImportedFunctions as ImportedFunctions[Name] extends Runnable ? Name : never]:
    ImportedFunctions[Name] extends Runnable<infer Data, infer Return> ? (data: Data) => Promise<Return>;
};

export function httpsCallables<ImportedFunctions>(functions: Functions) {
  type CallableFunctions = GetCallableFunctions<ImportedFunctions>;
  
  return <Name extends keyof CallableFunctions>(name: Name):
    CallableFunctions[Name] extends (data: infer Data) => infer Return ? HttpsCallable<Data, Return> : never = 
    httpsCallable(functions, name);
}

It would be nice if there was a slightly less hacky way to do this, ideally one where you didn't need to bring in your own helper for creating the callable functions, but that's out of scope for this repo/PR.

I'm currently hacking my way around this by overloading the https.onCall function with a type declaration, but it would be much nicer if I could get the data/return type directly without it.

No Breaking Changes

I added a default value for the Return type parameter, which should mean that this isn't a breaking change for anyone currently using Runnable.

Broken Tests

The changes here surfaced some type errors in some tests, and I went with a quick and dirty fix as it wasn't immediately obvious whether the tests are wrong, or whether they're just taking some type system shortcuts.

The data and resource fields on the event objects in spec/v1/providers/database.spec.ts don't match up with the definition of Event (from src/cloud-functions.ts).

Specifically, the data property should be an instance of DataSnapshot. I tried using const event: Event = { but then the context.resource field errors instead, expecting it to be a Resource (src/cloud-functions.ts) rather than a string.

Let me know if there's a better fix than any here and I'll happily amend the PR.

Thanks!

@google-cla google-cla bot added the cla: yes label Oct 17, 2021
@danprince danprince closed this by deleting the head repository Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant