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: Request/Response support, adapter cloudflare and streams #395

Closed
wants to merge 14 commits into from

Conversation

Hebilicious
Copy link
Member

@Hebilicious Hebilicious commented May 29, 2023

πŸ”— Linked issue

fix #394
fix #401
fix #144

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Why ?

  • Supporting streaming on all platforms.
  • Allowing users to conveniently use h3 in environment that aren't node
  • Returning Response objects from event handler in node environments

I needed these features on one of my project, and I'm contributing them back here.

event.request alternative API

Can be used when h3 is initialised with createEvent(undefined, undefined, request).
This API allows you to return native Response objects.
This is more natural in environment that aren't node, such as cloudflare, lagon or deno.

  const app = createApp();
  const router = createRouter();
  router.get(
    "/hello",
    eventHandler((event) => {
      const request = event.request; // Request object, from the fetch API
      return new Response(`hello ${request.method}`);
    })
  );
  app.use(router);
  const response = await app.handler(
    createEvent(
      undefined,
      undefined,
      new Request(new URL("http://localhost/hello"))
    )
  );
  console.log(await response.text()); // hello GET

Event handler Response support

Supports Response and RawResponse to override setters values.
Helpers functions are usable instead: sendResponse and sendResponseRaw.
This works in node too, the Response() will be converted and sent with res.write and res.end.

      eventHandler(() => {
        setHeader(event, "hello", "yes"); // will take priority 
        setResponseStatus(event, 207, "hello-status"); // will take priority 
        return new Response("valid", {
          status: 208,
          statusText: "bye-status",
          headers: { hello: "no" },
        });
      })
      eventHandler((event) => {
        setHeader(event, "hello", "yes");
        setResponseStatus(event, 208, "bye-status");
        return new RawResponse("valid", {
          status: 207, // will take priority 
          statusText: "hello-status", // will take priority 
          headers: { hello: "world" }, // will take priority 
        });
      })

Streaming support

Through Response, streaming can be achieved with TransformStream

      eventHandler(() => {
        const response = new Response(`Hello world !`);
        const { readable, writable } = new TransformStream();
        response.body?.pipeTo(writable);
        return new Response(readable, response);
      })

Nitro

This change makes a lot of sense with Nitro because the majority of the providers are Response based.
Given that Nitro is the main consumer of h3, here's what we could do :

  • abstract all references to event.node to a getter/setter method
  • add a nitro.response() call that doesn't use unenv/toNodeListener and always returns a Response.
  • update existing adapters/create new ones for the new syntax.

The following presets would now supports streaming, be simpler and more natural to use, and the nitro code would simplify as we could always return nitro.response() :

  • cloudflare modules
  • cloudflare service worker
  • cloudflare pages
  • vercel-edge (use cloudflare under the hood)
  • deno
  • deno-deploy
  • netlify-edge (uses deno-deploy under the hood)
  • lagon
  • service workers
  • bun

fetch adapter and cloudflare worker adapter

A new fetchAdapter to help use h3 in web standard environments.
For example with nitro, it can be used like this :

 const localResponse = adapterFetch(h3app)
   const app: NitroApp = {
   	// ...
	localResponse
  };
 // Then in a entry
 const response = nitroApp.localResponse(request, customContext)

To illustrate the use case, a cloudflare worker adapter is also provided:

const app = createApp();
const router = createRouter();

router.get( "/",  eventHandler((event) => new Response(`Hello world ! ${event.request.url}`)))

app.use(router);

const cloudflare = adapterCloudflareWorker(app);
export default cloudflare;

H3 can now be used easily with cloudflare workers, similarly to other frameworks like hono.

The internal API uses createEvent(undefined, undefined, request) instead of createEvent(nodeRequest, nodeResponse), and provide the alternative API for the platform who aren't node base.

Helper Caveats

Almost all of the helpers are supported universally, except these 3 :

  • writeEarlyHints
  • sendStream
  • isStream

They are too node specific and don't make sense for Request/Response platforms.
Every other helper is supported.

Additional thoughts

It looks like the respondWith helper could be deprecated as sendResponse replaces it.
@pi0 Please let me know if you think this PR should be split, altough it was very natural to do these changes together.
I could trim it by separating out the cloudflare adapter, the new helpers, the Response support and the event.request API.
(Altough it could easily be argued that the cloudflare adapter is just here for the tests)

πŸ“ Checklist

  • I have linked an issue or discussion.
  • h3 Native Response API support
  • Support all helpers
  • CF adapter: TransformStream support
  • CF adapter: Tests
  • Tests for event.request

@Hebilicious Hebilicious marked this pull request as draft May 29, 2023 10:52
@Hebilicious Hebilicious changed the title [POC]: adapter cloudflare and streams [POC]: Request/Response support, adapter cloudflare and streams Jun 6, 2023
@Hebilicious Hebilicious force-pushed the adapter-cloudflare-streams branch from a2d6a1f to 458103f Compare June 6, 2023 20:49
@Hebilicious Hebilicious marked this pull request as ready for review June 7, 2023 03:05
@Hebilicious Hebilicious changed the title [POC]: Request/Response support, adapter cloudflare and streams [WIP]: Request/Response support, adapter cloudflare and streams Jun 7, 2023
@Hebilicious Hebilicious changed the title [WIP]: Request/Response support, adapter cloudflare and streams feat: Request/Response support, adapter cloudflare and streams Jun 8, 2023
@Hebilicious Hebilicious marked this pull request as draft June 8, 2023 10:20
@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #395 (84acd6e) into main (6fd36cf) will decrease coverage by 0.49%.
The diff coverage is 81.04%.

❗ Current head 84acd6e differs from pull request most recent head 85ddc29. Consider uploading reports for the commit 85ddc29 to get more accurate results

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
- Coverage   76.21%   75.72%   -0.49%     
==========================================
  Files          26       29       +3     
  Lines        2438     2764     +326     
  Branches      339      403      +64     
==========================================
+ Hits         1858     2093     +235     
- Misses        580      671      +91     
Impacted Files Coverage Ξ”
src/utils/cache.ts 27.41% <11.76%> (-7.20%) ⬇️
src/utils/body.ts 73.09% <42.18%> (-19.71%) ⬇️
src/utils/session.ts 22.01% <50.00%> (+0.35%) ⬆️
src/utils/cookie.ts 88.19% <66.66%> (-4.80%) ⬇️
src/adapters.ts 76.19% <76.19%> (ΓΈ)
src/error.ts 76.02% <81.81%> (-0.20%) ⬇️
src/utils/response.ts 66.38% <82.70%> (+3.53%) ⬆️
src/utils/headers.ts 83.82% <83.82%> (ΓΈ)
src/router.ts 95.86% <85.71%> (+1.60%) ⬆️
src/utils/proxy.ts 82.38% <86.66%> (-0.51%) ⬇️
... and 9 more

@Hebilicious Hebilicious marked this pull request as ready for review June 13, 2023 17:38
@pi0
Copy link
Member

pi0 commented Jun 20, 2023

WoW lots of amazing stuff. Thank you so much for making a cumulative overview of your work. It is certainly the direction we want to go slowly to prefer web APIs over Node. (#73)

However, this amount of changes needs to be slowly adopted. I think fetch adapter would be a really good start.

I will keep in contact with you in Discord in upcoming days to have a call and plan on changes. ❀️

In the meantime closing PR to cleanup for maintenance. Probably also adding some small changes in this direction.

@pi0 pi0 closed this Jun 20, 2023
@Hebilicious
Copy link
Member Author

Hebilicious commented Jun 21, 2023

Looking forward for your call, in the meantime if anyone wants to try out this version that I'm personally using, you can find it on github and npm

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