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

feat: Added h3 server handler to @ingest/h3. #310

Merged
merged 4 commits into from
Sep 19, 2023

Conversation

michealroberts
Copy link
Contributor

@michealroberts michealroberts commented Sep 8, 2023

Summary

Added h3 server handler to @ingest/h3.

h3 is the unjs project which is the foundation for the Nuxt 3 server eventHandlers.

This MR provides backwards compatibility between Nuxt and h3, and can also be used within a nitro server setup too.

Includes bumping h3 to the latest version (v1.8.1).

Checklist

  • Added a docs PR that references this PR
  • Added unit/integration tests
  • Added changesets if applicable

@changeset-bot
Copy link

changeset-bot bot commented Sep 8, 2023

🦋 Changeset detected

Latest commit: 33687b8

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

@michealroberts
Copy link
Contributor Author

cc @pi0 Just pinging you here, because usage of the getRequestURL utility from h3 fails when used in place of the code in this MR ... it fails on this error:

 TypeError: Cannot read properties of undefined (reading 'encrypted')

Due to this line here: https://github.com/unjs/h3/blob/12ccb81432a62783fe951a8170976c8162bce44a/src/utils/request.ts#L129

Do you want me to add an issue to https://github.com/unjs/h3 ?

@pi0
Copy link

pi0 commented Sep 8, 2023

Hi. Haven’t tried locally but are you somehow mocking request and response objects passed to h3 habdler?

Regardless feel free to directly open a pr for h3 to make req.connection?. Check optional

@michealroberts
Copy link
Contributor Author

@pi0 Cool. MR here for you when ready: unjs/h3#532

@jpwilliams jpwilliams added the prerelease/inngest Create snapshot releases for a PR for the "inngest" package. label Sep 8, 2023
@jpwilliams jpwilliams self-requested a review September 11, 2023 19:37
Copy link
Member

@jpwilliams jpwilliams left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for the MR, @michealroberts!

One mini change to some tests and we can get this in. Even has a changeset already! 😍

feat: Added h3 server handler to @ingest/h3.
@jpwilliams jpwilliams self-assigned this Sep 13, 2023
@jpwilliams
Copy link
Member

@michealroberts I pushed a quick change to ensure handlers are still reported correctly for the benefit of the UI. It's a touch ugly, but this is changing soon anyway.

Could I grab an example for you for a typical short h3 setup? I'll add an example to the Serving the Inngest API docs. 🙂

@michealroberts
Copy link
Contributor Author

@michealroberts I pushed a quick change to ensure handlers are still reported correctly for the benefit of the UI. It's a touch ugly, but this is changing soon anyway.

Could I grab an example for you for a typical short h3 setup? I'll add an example to the Serving the Inngest API docs. 🙂

Cool, yeh I will add an example. It's exactly like Nuxt, but it can be a tad esoteric to know. This would also support nitro as well, which Nuxt is built on.

Kind of like turtles all the way down.

Nuxt 3 > Nitro > h3

@jpwilliams
Copy link
Member

@michealroberts Just a quick snippet is fine 🙂 Would something like this be reasonable?

import { createApp, eventHandler } from "h3";
import { serve } from "inngest/h3";

const app = createApp();

app.use(
  "/api/inngest",
  eventHandler(serve(...)),
);

@michealroberts
Copy link
Contributor Author

@jpwilliams Yaeh, exactly that!

Do you want the same again, but for nitro? https://nitro.unjs.io/

@jpwilliams jpwilliams merged commit 696c411 into inngest:main Sep 19, 2023
jpwilliams pushed a commit that referenced this pull request Sep 21, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## inngest@2.7.0

### Minor Changes

- [#313](#313)
[`32c34b3`](32c34b3)
Thanks [@jpwilliams](https://github.com/jpwilliams)! - Add ability to
declare and send events without payloads, e.g. `inngest.send({ name:
"my.event" });`

- [#310](#310)
[`696c411`](696c411)
Thanks [@michealroberts](https://github.com/michealroberts)! - Added h3
framework server handler

### Patch Changes

- [#319](#319)
[`71b7d26`](71b7d26)
Thanks [@jpwilliams](https://github.com/jpwilliams)! - Add ESM exports
to `inngest` package to avoid extension imports

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prerelease/inngest Create snapshot releases for a PR for the "inngest" package.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants