-
Notifications
You must be signed in to change notification settings - Fork 43
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
INN-998 Add local Zod schemas when instantiating clients #66
Conversation
a775c8a
to
78e5c81
Compare
Hey @jpwilliams - excited about first class Zod support. Here are my thoughts on this topic and the potential UX:
const MyZodEvent = z.object({
name: z.literal("my.zod.event"),
data: z.object({
z.string(),
})
});
type APIEvents = API_ItemCreated | API_ItemDeleted;
const new Inngest({
schemas: [
MyZodEvent
new Schema<EventA | EventB>(),
new Schema<APIEvents>(),
new Schema<GeneratedEvents>()
new Schema()
]
})
import { stripeSchemas } from "@inngest/schemas";
const new Inngest({ schemas: [ new Schemas<MyCustomEventA>(), stripeSchemas ] });
I'm not set on the best UX, but thinking through some of the above might allow us to think about something that gives us future flexibility. |
@djfarrelly The upside of a class with chainable methods is that it's easier to both discover and support new methods of defining schemas. Suppose the With the chainable methods, every method is discoverable via autocomplete, is responsible only for its own simpler roll-up of the given type to an expected standard type, and errors will be much easier to read as they are solely focused on a single format.
@djfarrelly 💯 Like you, I'm not a fan of introducing more concepts where it feels like a user could just define an object, though I think the benefits are worth it. Already with just supporting Zod, TS types, and generated schemas, the Hopefully with this, we make it clear that users can define a schemas with a
@djfarrelly Agreed - it's likely some users will want their schemas held away from the client to not muddy that file. Chainable methods will support this pattern well, allowing you to build up your schemas from multiple imported sources.
@djfarrelly Yes! One common improvement I can see is being able to set schema-specific options up front. For example, some users might like the ability to build an untyped function before specifying their event. For this, we could provide something like const schemas = new Schemas()
.fromTypes<{
"app/user.created": {
data: { id: number };
};
}>()
.allowUntyped(); Could also be an option passed to
@djfarrelly Absolutely! This would be a great way to grab many known types without forcing them on new users. We could even bundle this into the SDK itself, so there's not a separate package; seeing as they're just types they'll never be shipped so resulting package size isn't an issue, though general package download size for the developer might bloat a bit.
@djfarrelly Yep, we can support this pattern! My only reservation is that you can accidentally provide events with the same name which might change some of the internal typing. When a user does this and then declares { ok: boolean } | { id: string } I guess in these cases we'd also want to enforce a if (event.v === "2") {
// data is typed correctly for this particular event
} else {
// only one other option, so typed correctly again
} |
This would be really good, the automatic schema generation makes a good demo but I really want things static and defined in code. I'm currently using zod for this which works today. const data = z.object({
a: z.string()
})
type Events = {
"event1": {
name: "event1";
data: z.infer<typeof data>;
};
}; Then I parse the event into Zod in the function, having this built in sounds great. PS having to write out the event name twice is frustrating. |
e242f29
to
eff8f4e
Compare
In addition to the above requirements, supporting wildcards is a big win for many systems, e.g. listening to Given the stacking set of schemas using For example, the following stacked schema: const inngest = new Inngest({
name: "My Client",
schemas: new Schemas()
.fromTypes<{
"app/user.created": {
data: { id: number };
};
}>()
.fromZod({
"app/user.deleted": {
data: z.object({ id: z.number() }),
},
"app/post.created": {
data: z.object({ id: z.number() }),
},
}),
}); Would produce the following types for listening: type Events = {
"app/user.created": AppUserCreated;
"app/user.deleted": AppUserDeleted;
"app/post.created": AppPostCreated;
"app/*": AppUserCreated | AppUserDeleted | AppPostCreated;
"app/user.*": AppUserCreated | AppUserDeleted;
"app/post.*": AppPostCreated;
"app/*.created": AppUserCreated | AppPostCreated;
"app/*.deleted": AppUserDeleted;
"*": AppUserCreated | AppUserDeleted | AppPostCreated;
"*/user.*": AppUserCreated | AppUserDeleted;
"*/post.*": AppPostCreated;
"*/*.created": AppUserCreated | AppPostCreated;
"*/*.deleted": AppUserDeleted;
}; We might need a A separate feature is also planned to be able to specify multiple triggers which would produce a similar union. Post V1, the trigger section will be To specify multiple events, we'd just be passing an array instead. inngest.createFunction(
"My Function",
["app/user.created", { event: "app/user.updated" }, { cron: "0 9 * * MON" }]
async ({ event }) => {
// typeof event is (AppUserCreated | AppUserUpdated | null)
}
); |
…ean examples (#45) ## Breaking changes ### Function creation helpers removed The following function creation methods have been removed and should be replaced by using `Inngest#createFunction`: - `createFunction` - `createScheduledFunction` - `createStepFunction` - `Inngest#createScheduledFunction` - `Inngest#createStepFunction` ```ts import { createFunction } from "inngest"; createFunction("Example", "app/user.created", () => {}); // becomes import { Inngest } from "inngest"; const inngest = new Inngest({ name: "App" }); inngest.createFunction("Example", "app/user.created", () => {}); ``` ```ts import { createScheduledFunction } from "inngest"; createScheduledFunction("Example", "* * * * *", () => {}); // or inngest.createScheduledFunction // becomes import { Inngest } from "inngest"; const inngest = new Inngest({ name: "App" }); inngest.createFunction("Example", { cron: "* * * * *" }, () => {}); ``` ```ts import { createStepFunction } from "inngest"; createStepFunction("Example", "* * * * *", () => {}); // or inngest.createStepFunction // becomes import { Inngest } from "inngest"; const inngest = new Inngest({ name: "App" }); inngest.createFunction("Example", { cron: "* * * * *" }, async ({ tools }) => { // Use a tool to create a step function }); ``` ### Step functions are now asynchronous In order to provide the full power of asynchronous JavaScript, step functions are now required to be async functions, and all step tooling will return promises instead of synchronously. If you're using TypeScript, you'll be guided through the changes at each stage. For example, trying to access `.id` of the new `Promise<User>` will throw an error telling you that it must first be awaited. ```ts import { createStepFunction } from "inngest"; import { userDb } from "./db"; import { email } from "./email"; export default createStepFunction( "Example", "app/user.created", ({ event, tools }) => { const user = tools.run("Get user email", () => userDb.get(event.userId)); // We run synchronously, so wait for the email to be send before the alert is sent tools.run("Send email", () => email.sendEmail(user.email, "Welcome!")); tools.run("Send alert to staff", () => email.sendAlert("New user created!") ); } ); ``` This would be converted to the following: ```ts import { inngest } from "./client"; import { userDb } from "./db"; import { email } from "./email"; export default inngest.createFunction( // use client instead of helper { name: "Example", fns: { ...userDb, ...email } }, // can pass functions to wrap in tools.run() "app/user.created", async ({ event, fns: { getUser, sendEmail, sendAlert }}) => { const user = await getUser(event.userId); // use fns directly that have been passed // We don't await these, so they are run in parallel now sendEmail(user.email, "Welcome!"); sendAlert("New user created!"); } ); ``` ### Custom handlers require a `stepId` In order to provide the parallel functionality in this PR, all handlers created using `InngestCommHandler` must provide a `stepId` parameter when attempting to run a function. This should be accessed via the query string using the exported `queryKeys.StepId` enum. ```diff run: async () => { if (req.method === "POST") { return { fnId: url.searchParams.get(queryKeys.FnId) as string, + stepId: url.searchParams.get(queryKeys.StepId) as string, ``` ## Features ### Pass functions to wrap as retriable steps When creating a function, you can now pass a `fns` key to automatically wrap all found functions in `tools.run()` step tooling, automatically providing your existing functionality with retries and durability. ```ts import { inngest } from "./client"; import { userDb } from "./db"; import { email } from "./email"; export default inngest.createFunction( { name: "Example", fns: { ...userDb, ...email } }, "app/user.created", async ({ event, fns: { getUser, sendEmail}}) => { const user = await getUser(event.userId); sendEmail(user.email, "Welcome!"); } ); ``` ## Fixes - `user` key in event payloads can now be any value, to ensure conflicting generated events are accepted; see #87 for further discussion ## Related changes/issues based on this PR - #66 - #73 - #74 - #75 - #76 - #69 - #43 ## SDK changes - [x] Refactor step function tooling to be `async` - [x] Unify `createFunction`, `createScheduledFunction`, and `createStepFunction` under a single `createFunction` method - [x] ~Preserve complex client-less `createFunction` helper~ > Prefer instantiation with a client for now. - [x] Add error-handling capabilities by giving the SDK the ability to send back a serialised `error` as well as `data` - [x] Allow use of regular JS tooling (`.then()`, `try/catch`, etc) in step functions without gotchas - [x] ~Create a new `StepOpCode` for a no-op other than `None`~ We just return an empty array. - [x] Add ability to pass steps in as "just functions" to avoid wrapping all user code in `tools.run()` - [x] ~Add ability to pass steps in as "just functions" to a `new Inngest()` client, which is merged with any steps passed directly to `inngest.createFunction()`~ > Defer this to be implemented with #66, as this will alter generics for `Inngest` and `InngestFunction` to aid with this change. - [x] Hash synchronous groups of steps and throw errors if synchronous step order doesn't match - [x] Allow triggering steps using `stepId` query param instead of op position codes - [x] Add new `StepPlanned` (or similar) op code for back compat - [ ] Create `step` alias for `tools` and deprecate `tools` - [ ] Add a large warning when the a step function resolves, but some tooling is still pending; we want to highlight for users that to be safe they should make sure to await in serverless environments ## Examples <details> <summary>See examples</summary> ```ts // Any type of function uses the same method. // declare createFunction: (nameOrFunctionOpts, eventNameOrTriggerOpts, fn) => void; inngest.createFunction("...", "demo/event.sent", () => {}); inngest.createFunction("...", { event: "demo/event.sent" }, () => {}); inngest.createFunction("...", { cron: "* * * * *" }, () => {}); ``` ```ts // A step function uses the same method; just use a tool. inngest.createFunction("...", "demo/event.sent", ({ tools: { run } }) => { run("Step", () => {}); }); ``` ```ts // Step functions now use async functions - easier to map complex flows. inngest.createFunction("...", "demo/event.sent", async ({ tools: { run } }) => { const randomNumber = await run("Get random number", () => Math.random()); await run("Send random number", () => sendEmail("...", randomNumber)); }); ``` ```ts // With async support, fire-and-forget parallel steps are easy to create - just // trigger them all and SDK will let Inngest know of all of the pending actions. inngest.createFunction("...", "demo/event.sent", async ({ tools: { run } }) => { run("Send email", () => sendEmail("...")); run("Send another email", () => sendEmail("...")); run("Send yet another email", () => sendEmail("...")); }); // This means that the response from the SDK to Inngest is always an array of // upcoming steps. // // In this case, the first call would return all three actions, as we're not // awaiting them; the SDK gets a single synchronous tick of the event loop to // decide what is next. [ { op: "Step", name: "Send email", run: true }, { op: "Step", name: "Send another email", run: true }, { op: "Step", name: "Send yet another email", run: true }, ]; ``` ```ts // Step functions can handle errors too, meaning we can use usual tools like // try/catch. inngest.createFunction("...", "demo/event.sent", async ({ tools: { run } }) => { try { await run("Step", () => {}); } catch (err) { await run("Send error to Tim", () => sendEmail("...", err)); } }); // Or perhaps we want to silently handle an error case and not have it affect // other steps. inngest.createFunction("...", "demo/event.sent", async () => { await run("Send email", () => sendEmail("...")).catch(() => { run("Send error to Tim", () => sendEmail("...", err)); }); }); ``` ```ts // Tools such as `Promise.all` are fine too. inngest.createFunction("...", "demo/event.sent", async ({ tools: { run } }) => { await Promise.all([ run("Send email", () => sendEmail("...")), run("Send another email", () => sendEmail("...")), ]); }); ``` ```ts // We can even create parallel chains of steps that trigger immediately, follow // their own path, then eventually come back together. inngest.createFunction("...", "demo/event.sent", async ({ tools: { run } }) => { const fooStep = run("Foo", () => fooSomething()) .then((data) => run("Foo again", () => fooSomethingElse(data))) .then((data) => run("Foo again again", () => fooSomethingElse(data))); const barStep = run("Bar", () => barSomething()) .then((data) => run("Bar again", () => barSomethingElse(data))) .then((data) => run("Bar again again", () => barSomethingElse(data))); const [foo, bar] = await Promise.all([fooStep, barStep]); await run("Send email", () => sendEmail("...", { foo, bar })); }); ``` ```ts // Wrapping all user code in `run()` can be a bit verbose. One of the initial // goals of the SDK was that steps were "just functions". If we pass those to // our Inngest function then we can shim them with `run()` automatically, which // makes the code look _super_ clean. // // The TS types and runtime JS will non-destructively filter the input and add // shims, meaning you could pass entire structures/classes in with no bother, // like we do here with `userDb` and `email` which may export more than just // functions. We even retain the function's comments! import * as userDb from "./dbs/user"; import * as email from "./email"; inngest.createFunction( { name: "...", fns: { ...email, ...userDb } }, "demo/event.sent", async ({ event, fns: { sendEmail, getUserById } }) => { const user = await getUserById(event.user.id); await sendEmail(user.email, "..."); } ); // This can really help clean up functions with lots of steps, like our example // of parallel paths above. inngest.createFunction( { name: "...", fns: { ...fooLib, ...barLib, ...email } }, "demo/event.sent", async ({ tools: { run } }) => { const fooStep = fooSomething() .then(fooSomethingElse) .then(fooSomethingElse); const barStep = barSomething() .then(barSomethingElse) .then(barSomethingElse); const [foo, bar] = await Promise.all([fooStep, barStep]); await sendEmail("...", { foo, bar }); } ); ``` ```ts // Sometimes, a user may want to bundle async actions together in a single step, // for example when something must be fetched from the DB right before sending // an email. // // If this is unique to this Inngest function, we can always use `tools.run()` // to run in-line code. If we wanted to create a reusable step, however, we just // make a regular function. const sendEmailToUser = async (userId: string, body: string) => { const user = await getUserById(userId); await sendEmail(user.email, body); }; inngest.createFunction( { name: "...", fns: { sendEmailToUser, something } }, "demo/event.sent", async ({ event }) => { await something(); await sendEmailToUser(event.user.id, "..."); } ); ``` ```ts // If we were to create a library of reusable steps, a _future_ option that is // not in this PR could be to pass steps in to the Inngest constructor to // provide the tooling for every function automatically. const inngest = new Inngest<Events>({ name: "...", fns: { ...email, ...userDb, ...postDb }, }); inngest.createFunction( "...", "demo/event.sent", async ({ event, fns: { getUserById, getPostsByTag, sendEmail } }) => { const user = await getUserById(event.user.id); const posts = await getPostsByTag(user.favouriteTag); await sendEmail(user.email, posts); } ); ``` </details>
eb62ce0
to
8207488
Compare
Will also consider ArkType for a schema candidate. See arktypeio/arktype#687. |
8207488
to
6589913
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! Excited about this change — it opens the door for a lot!
@tonyhb @jpwilliams whats the best way to test this out? do you plan to release rc/alpha versions of the sdk so we can get our hands dirty testing this out? |
This PR is currently available to test out by installing It's a breaking change, so working on bundling together a few breaking changes for 2.0 before shipping. :) |
@jpwilliams thanks for the bump. this comes right when I needed it 👍 will keep you posted if anything comes up while testing it out |
@jpwilliams this is working great so far 👍 no issues. currently using the |
Awesome to hear! 🙂 Thanks for reporting back. Any preference in regard to |
@jpwilliams I think this is just preference. for easy examples in the docs I'd says that the union types are more expressive as you don't need to repeat function names everywhere. I don't know (yet) if I'll use the other options ( |
Closing this PR as it's shifting to #203. Will include release notes for this feature there. |
Summary - INN-998
Same-app types should be easily specified within the codebase, without having to wait for data to flow to production and then generating types.
Also, nobody likes code generation, so if we can avoid requiring that, then everyone's happier.
This PR introduces the ability to utilise colinhacks/zod to specify typing.
If you want to specify types using just TS types instead, a function is provided to let you do that.
I'd love to provide this as a top-level generic, so we can do
new Inngest<TYPES>()
as we can now, but this locks us in a nasty place. We use a lot of inference across the client, but TS still doesn't support inferring only some generics - see microsoft/TypeScript#26242; if we enabled this, then we'd no longer be able to infer functionality or tooling from options choices smartly in the future.An alternative we could explore could be to create a
Schemas
class, where the API would allow us to build and merge our types from varying sources, e.g.:This feels a touch messier to get started but provides a lot more power in the long run, as we could merge generated, TS, and Zod typing as one, choosing overwriting order. In addition, discoverability is great here, allowing users to see the
fromZod()
function during creation to understand they can use Zod types without having to read comments or docs.Related
async
/await
parallel steps, unifycreateFunction
, clean examples #45