Skip to content

Add weekly digest function #1

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

djfarrelly
Copy link

This extends the initial Inngest implementation by adding a weekly digest function for the Notes app. This first function is meant to show how to run something asynchronously using Inngest. A cron is the most basic example.

Improvements

I was tempted to extend this with additional best practices, but held off prior to getting Kent's thoughts. If this were production-grade the cron function would use step.invoke() or step.sendEvent() to fan-out the actual email sending to another function. This would two functions each with a single responsibility:

  1. a cron function to load all users that have a digest enabled and
  2. a function that fetches the digest data for each user in the database then formats the email and sends it.

The benefits of that approach is that the second function can then use flow control, such as throttle to match Resend (or whatever provider's) API rate limit. Resend, by default limits to 2 requests per second, so sending a digest to many users so quickly may easily result in an 429 response from Resend's API. In the demo this isn't bad since emails aren't actually being sent, and just being printed to stdout, but in production, you would 100% want to add throttling.

Test Plan

Start up the server as typical with npm run dev then in another terminal, run the Inngest dev server pointing to the route:

npx inngest-cli@latest dev -u http://localhost:3000/resources/inngest --no-discovery

Note - --no-discovery disables Inngest app port and path scanning that is helpful when originally getting started with Inngest.

Notes

  • Best practices: Typically, we recommend that any Inngest route be served at /api/inngest,
  • DX: I would add the npx inngest-cli dev command as an npm run script and potentially run this concurrently. This could also be achieved by wrapping the CLI and executing it via child process in dev-server.js

Checklist

  • Tests updated
  • Docs updated

Screenshots

Function available in the Inngest dashboard:
image

Test run of the function using local data:
image

@kentcdodds
Copy link
Member

This is so awesome. Thank you for putting it together! I'm excited to try it out soon.

Am I missing something or does the 'note.created' never fire? Should we put that in our remix app code when notes are created?

@djfarrelly
Copy link
Author

Good catch @kentcdodds. I added that event originally when I anticipated doing a non-cron, event-triggered function, but that would have been an expansion of functionality. I think that's the more interesting thing, but would love to bounce ideas off you of what would work in the context of this application that feels good and approachable for folks learning. Here are some ideas:

  • note.created triggers a function that summarizes the note via LLM and adds a "summary" field to the note itself in a new column
  • note.created triggers a function that "sleeps" until the next morning then sends the user a nudge like "You're on a n day streak, create another note today"

I'm also happy to write any other idea that you might have. I think having a cron and a typical background job shows the two main categories for folks that they might need and then can expand from there! Eager to hear your thoughts!

@kentcdodds
Copy link
Member

Sorry for taking so long on this. Epic React v2 launch and traveling all over the world the last couple weeks has taken all of my time 😅

I like both of those ideas!

I feel like the n day streak idea could feature more of what inngest is capable of. Like, maybe we could only send the email when they're about to lose their streak or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants