inngest / inngest-js

The developer platform for easily building reliable workflows with zero infrastructure for TypeScript & JavaScript
https://www.inngest.com/
GNU General Public License v3.0
414 stars 41 forks source link

Allow specifying `function` with `appId` when invoking #449

Closed jpwilliams closed 8 months ago

jpwilliams commented 9 months ago

Summary

When using step.invoke(), the provided function option can be either an InngestFunction instance or a string.

In the latter case, the user is required to also include the ID of the app being called, which is unclear. This PR makes a few changes to accommodate this nicety.

Awkwardly, the functionality of function: string must stay the same to avoid a breaking change. This makes adding a new functionId option a bit confusing, as now I can specify both function: string and functionId: string.

A related issue for invocation is wanting to import only a function's types to avoid also importing all of another function's dependencies when invoking. This also applies to cross-app invocation, where a user may want to invoke a function using a string ID and simultaneously provide typing or schemas for validating input/output.

This change requires some form of referencing a function using only an ID and relying on either the types of an InngestFunction instance or some inputted types/schemas.

Broadly, we add a new referenceFunction export (I don't like the name of it) which can be passed in as the function being invoked. We should also deprecate use of function: string and prefer creating a referenceFunction.

Currently, and unchanged to avoid a breaking change

A "full" function ID, which is [clientId]-[functionId]. Input and output types are unknown.

await step.invoke("start-process", {
  function: "some-python-app-some-python-fn",
});

Currently, and unchanged

Input and output types are known.

import { someInngestFn } from "@/inngest/someFn";

await step.invoke("start-process", {
  function: someInngestFn,
});

Referencing a function using an existing function instance

No dependencies of the target function are imported.

The function ID must be provided at runtime, but we can enforce that this is the literal ID given to the function, i.e. functionId: "some-fn" instead of just functionId: string.

Input and output types are known.

import { referenceFunction } from "inngest";
import { type someInngestFn } from "@/inngest/someFn";

await step.invoke("start-process", {
  function: referenceFunction<typeof someInngestFn>({
    functionId: "some-fn",
  }),
});

Referencing some external function by ID

The app ID of the client executing the call is used, as we don't specify an appId. Input and output types are unknown.

import { referenceFunction } from "inngest";

await step.invoke("start-process", {
  function: referenceFunction({
    functionId: "some-fn",
  }),
});

Referencing some external function by ID and app ID

We specify an app ID here to form the full ID instead of using the executing client's ID. Input and output types are unknown.

import { referenceFunction } from "inngest";

await step.invoke("start-process", {
  function: referenceFunction({
    functionId: "some-fn",
    appId: "some-app",
  }),
});

Specifying types for the input and output of a reference function

We can also provide input and output schemas to add typing to our reference function, which adds types to the required input and output of step.invoke().

Providing a schema is optional and you can also choose to provide only an input or only an output.

[!NOTE] If you've passed an actual Inngest function like referenceFunction<typeof someFn>({ ... }), you cannot add additional schemas.

For the purposes of this example, we use Zod, but this will expand to allow many validation libraries (or types directly) in the future. This will complement being able to provide schemas on a function directly in the future, as well as validating I/O instead of only using types.

import { referenceFunction } from "inngest";
import { z } from "zod";

await step.invoke("start-process", {
  function: referenceFunction({
    functionId: "some-fn",
    appId: "some-app",
    schemas: {
      data: z.object({
        foo: z.string(),
      }),
      return: z.object({
        success: z.boolean(),
      }),
    },
  }),
});

Reusing reference functions

It's likely healthy for a user to declare reference functions the same way they do all of their others, meaning importing and invoking it is the same experience for both local and remote functions.

For example, if a local function is declared like so:

// src/inngest/localFn.ts
import { inngest } from "@/inngest";

export default inngest.createFunction(/* ... */);

Then a remote function may be declared like so:

// src/inngest/remoteFn.ts
import { referenceFunction } from "@/inngest";

export default referenceFunction(/* ... */);

In both instances, the function is then simply imported and invoked.

// src/inngest/someFn.ts
import { inngest } from "@/inngest";
import { localFn } from "@/inngest/localFn";
import { remoteFn } from "@/inngest/remoteFn";

export default inngest.createFunction(
  { id: "some-fn" },
  { event: "some-event" },
  async ({ step }) => {
    return Promise.all([
      step.invoke({ function: localFn }),
      step.invoke({ function: remoteFn }),
    ]);
  },
);

Checklist

Related

changeset-bot[bot] commented 9 months ago

🦋 Changeset detected

Latest commit: 8ea53e2fb7c97b1ec74a4ffd323c331e51a81f5d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------- | ----- | | inngest | Minor |

Not sure what this means? Click here to learn what changesets are.

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

goodoldneon commented 9 months ago

Is this a breaking change for users already specifying function: appID + "-" + functionID?

tonyhb commented 9 months ago

Is this a breaking change for users already specifying function: appID + "-" + functionID?

https://github.com/inngest/inngest-js/pull/449/files#diff-498e58b5ce5bcba89da57c0c48881bb031a6526cf06b8af45e72deeb71644316R410-R412

Looks like this filters out empty strings before joining to make it backwards compatible

goodoldneon commented 9 months ago

Is this a breaking change for users already specifying function: appID + "-" + functionID?

https://github.com/inngest/inngest-js/pull/449/files#diff-498e58b5ce5bcba89da57c0c48881bb031a6526cf06b8af45e72deeb71644316R410-R412

Looks like this filters out empty strings before joining to make it backwards compatible

But client.id will always be a string, right? Seems like setting function to the full-qualified ID (with app ID) will result in a string twice prefixed with the app ID