Repository: unjs/h3
Pull request #506 (2023-08-14 17:04:08)

refactor: cleanup event interface
<!---
☝️ PR title should follow conventional commits (https://conventionalcommits.org) 
-->

### 🔗 Linked issue

<!-- Please ensure there is an open issue and mention its number as #123 -->

### ❓ Type of change

<!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply. -->

- [ ] 📖 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

Before releasing h3 1.8, this PR makes sure we minimize new interface changes and chances of future possible breaking changes.

Meanwhile, I was trying new Web APIs, and figured it is so easy to shoot in the foot by runtime expensive getters such as `event.url` or `event.request` (did myself ironically!). Composable utils are a little bit more verbose to use but also easy to replace or deprecate.

Changes from the last RC:

- Move `event.rawBody` to `getRequestWebStream(event)`
- Move `event.request` to `toWebRequest(event)`
- Move top-level web context to `event.web?` (`WebEventContext = { request, url }`)
- Drop `event._originalPath` internal getter
- Drop `event._hasBody` internal getter
- Move `event.url` to `getRequestURL(event)`
- Fixed an unseen issue with `readRawBody` and `readBody` to support web request body streams
- Simplify `getRequestPath` to only normalize trailing double slashes 
- Revert `getRequestPath` to 1.7.1 implementation and deprecate

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] I have linked an issue or discussion.
- [ ] I have updated the documentation accordingly.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) Report
> Merging [#506](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) (c6b92c1) into [main](https://app.codecov.io/gh/unjs/h3/commit/588c8a3d9b1e408f36a1f51a43d759534245029f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) (588c8a3) will **decrease** coverage by `1.14%`.
> The diff coverage is `87.50%`.

```diff
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
- Coverage   82.40%   81.26%   -1.14%     
==========================================
  Files          31       31              
  Lines        3580     3587       +7     
  Branches      531      531              
==========================================
- Hits         2950     2915      -35     
- Misses        630      672      +42     
```


| [Files Changed](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs) | Coverage Δ | |
|---|---|---|
| [src/utils/proxy.ts](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL3V0aWxzL3Byb3h5LnRz) | `83.81% <50.00%> (ø)` | |
| [src/adapters/plain.ts](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL2FkYXB0ZXJzL3BsYWluLnRz) | `79.35% <66.66%> (+0.13%)` | :arrow_up: |
| [src/adapters/web.ts](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL2FkYXB0ZXJzL3dlYi50cw==) | `94.82% <66.66%> (ø)` | |
| [src/event/event.ts](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL2V2ZW50L2V2ZW50LnRz) | `86.61% <86.84%> (-1.96%)` | :arrow_down: |
| [src/utils/body.ts](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL3V0aWxzL2JvZHkudHM=) | `94.23% <89.36%> (-1.22%)` | :arrow_down: |
| [src/utils/request.ts](https://app.codecov.io/gh/unjs/h3/pull/506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs#diff-c3JjL3V0aWxzL3JlcXVlc3QudHM=) | `93.12% <94.73%> (+0.34%)` | :arrow_up: |

... and [2 files with indirect coverage changes](https://app.codecov.io/gh/unjs/h3/pull/506/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=unjs)



Patch:
diff --git a/src/adapters/plain.ts b/src/adapters/plain.ts
index 7a69ae6..64b5136 100644
--- a/src/adapters/plain.ts
+++ b/src/adapters/plain.ts
@@ -5,6 +5,7 @@ import type { HTTPMethod } from "../types";
 import { createError, isError, sendError } from "../error";
 import { H3Event, createEvent, eventHandler } from "../event";
 import {
+  getRequestWebStream,
   setResponseHeader,
   setResponseStatus,
   splitCookiesString,
@@ -44,7 +45,7 @@ export function fromPlainHandler(handler: PlainHandler) {
       method: event.method,
       path: event.path,
       headers: Object.fromEntries(event.headers.entries()),
-      body: event.rawBody,
+      body: getRequestWebStream(event),
       context: event.context,
     });
     setResponseStatus(event, res.status, res.statusText);
@@ -82,7 +83,7 @@ export async function _handlePlainRequest(app: App, request: PlainRequest) {
   event._path = path;
   event._headers = headers;
   if (request.body) {
-    event._body = request.body;
+    event._requestBody = request.body;
   }
   if (request._eventOverrides) {
     Object.assign(event, request._eventOverrides);
diff --git a/src/adapters/web.ts b/src/adapters/web.ts
index fe353a3..07b2323 100644
--- a/src/adapters/web.ts
+++ b/src/adapters/web.ts
@@ -1,5 +1,6 @@
 import type { App } from "../app";
 import { eventHandler } from "../event";
+import { toWebRequest } from "../utils";
 import { _handlePlainRequest } from "./plain";
 
 /** @experimental */
@@ -19,7 +20,7 @@ export function toWebHandler(app: App) {
 
 /** @experimental */
 export function fromWebHandler(handler: WebHandler) {
-  return eventHandler((event) => handler(event.request, event.context));
+  return eventHandler((event) => handler(toWebRequest(event), event.context));
 }
 
 // --- Internal ---
@@ -34,8 +35,7 @@ async function _handleWebRequest(
   const url = new URL(request.url);
   const res = await _handlePlainRequest(app, {
     _eventOverrides: {
-      _request: request,
-      _url: url,
+      web: { request, url },
     },
     context,
     method: request.method,
diff --git a/src/event/event.ts b/src/event/event.ts
index a816976..297521e 100644
--- a/src/event/event.ts
+++ b/src/event/event.ts
@@ -1,21 +1,18 @@
 import type { IncomingHttpHeaders } from "node:http";
 import type { H3EventContext, HTTPMethod, EventHandlerRequest } from "../types";
 import type { NodeIncomingMessage, NodeServerResponse } from "../adapters/node";
-import { getRequestURL, sendWebResponse } from "../utils";
-
-// TODO: Dedup from body.ts
-const PayloadMethods: Set<HTTPMethod> = new Set([
-  "PATCH",
-  "POST",
-  "PUT",
-  "DELETE",
-]);
+import { sendWebResponse } from "../utils";
 
 export interface NodeEventContext {
   req: NodeIncomingMessage & { originalUrl?: string };
   res: NodeServerResponse;
 }
 
+export interface WebEventContext {
+  request?: Request;
+  url?: URL;
+}
+
 export class H3Event<
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   _RequestT extends EventHandlerRequest = EventHandlerRequest,
@@ -24,16 +21,15 @@ export class H3Event<
   "__is_event__" = true;
 
   // Context
-  node: NodeEventContext;
-  context: H3EventContext = {};
+  node: NodeEventContext; // Node
+  web?: WebEventContext; // Web
+  context: H3EventContext = {}; // Shared
 
   // Request
-  _request: Request | undefined;
-  _method: HTTPMethod | undefined;
-  _headers: Headers | undefined;
-  _path: string | undefined;
-  _url: URL | undefined;
-  _body: BodyInit | undefined;
+  _method?: HTTPMethod;
+  _path?: string;
+  _headers?: Headers;
+  _requestBody?: BodyInit;
 
   // Response
   _handled = false;
@@ -42,30 +38,7 @@ export class H3Event<
     this.node = { req, res };
   }
 
-  get _originalPath() {
-    return this.node.req.originalUrl || this.node.req.url || "/";
-  }
-
-  get _hasBody() {
-    return PayloadMethods.has(this.method!);
-  }
-
-  get path() {
-    return this._path || this.node.req.url || "/";
-  }
-
-  get url() {
-    if (!this._url) {
-      this._url = getRequestURL(this);
-    }
-    return this._url;
-  }
-
-  get handled(): boolean {
-    return (
-      this._handled || this.node.res.writableEnded || this.node.res.headersSent
-    );
-  }
+  // --- Request ---
 
   get method(): HTTPMethod {
     if (!this._method) {
@@ -76,6 +49,10 @@ export class H3Event<
     return this._method;
   }
 
+  get path() {
+    return this._path || this.node.req.url || "/";
+  }
+
   get headers(): Headers {
     if (!this._headers) {
       this._headers = _normalizeNodeHeaders(this.node.req.headers);
@@ -83,51 +60,12 @@ export class H3Event<
     return this._headers;
   }
 
-  /** @deprecated Please use `event.node.req` instead. **/
-  get req() {
-    return this.node.req;
-  }
+  // --- Respoonse ---
 
-  /** @deprecated Please use `event.node.res` instead. **/
-  get res() {
-    return this.node.res;
-  }
-
-  /** @experimental */
-  get rawBody() {
-    if (!this._hasBody) {
-      return undefined;
-    }
-    if (this._body === undefined) {
-      this._body = new ReadableStream({
-        start: (controller) => {
-          this.node.req.on("data", (chunk) => {
-            controller.enqueue(chunk);
-          });
-          this.node.req.on("end", () => {
-            controller.close();
-          });
-          this.node.req.on("error", (err) => {
-            controller.error(err);
-          });
-        },
-      });
-    }
-    return this._body;
-  }
-
-  /** @experimental */
-  get request(): Request {
-    if (!this._request) {
-      this._request = new Request(this.url, {
-        // @ts-ignore Undici option
-        duplex: "half",
-        method: this.method,
-        headers: this.headers,
-        body: this.rawBody,
-      });
-    }
-    return this._request;
+  get handled(): boolean {
+    return (
+      this._handled || this.node.res.writableEnded || this.node.res.headersSent
+    );
   }
 
   respondWith(response: Response | PromiseLike<Response>): Promise<void> {
@@ -136,13 +74,27 @@ export class H3Event<
     );
   }
 
+  // --- Utils ---
+
   toString() {
-    return `[${this.method}] ${this.url}`;
+    return `[${this.method}] ${this.path}`;
   }
 
   toJSON() {
     return this.toString();
   }
+
+  // --- Deprecated ---
+
+  /** @deprecated Please use `event.node.req` instead. **/
+  get req() {
+    return this.node.req;
+  }
+
+  /** @deprecated Please use `event.node.res` instead. **/
+  get res() {
+    return this.node.res;
+  }
 }
 
 export function isEvent(input: any): input is H3Event {
diff --git a/src/utils/body.ts b/src/utils/body.ts
index 21b361f..e783967 100644
--- a/src/utils/body.ts
+++ b/src/utils/body.ts
@@ -4,7 +4,7 @@ import type { Encoding, HTTPMethod, InferEventInput } from "../types";
 import type { H3Event } from "../event";
 import { createError } from "../error";
 import { parse as parseMultipartData } from "./internal/multipart";
-import { assertMethod, getRequestHeader } from "./request";
+import { assertMethod, getRequestHeader, toWebRequest } from "./request";
 import { ValidateFunction, validateData } from "./internal/validate";
 
 export type { MultiPartData } from "./internal/multipart";
@@ -35,6 +35,8 @@ export function readRawBody<E extends Encoding = "utf8">(
 
   // Reuse body if already read
   const _rawBody =
+    event._requestBody ||
+    event.web?.request?.body ||
     (event.node.req as any)[RawBodySymbol] ||
     (event.node.req as any).body; /* unjs/unenv #8 */
   if (_rawBody) {
@@ -42,6 +44,38 @@ export function readRawBody<E extends Encoding = "utf8">(
       if (Buffer.isBuffer(_resolved)) {
         return _resolved;
       }
+      if (typeof _resolved.pipeTo === "function") {
+        return new Promise<Buffer>((resolve, reject) => {
+          const chunks: Buffer[] = [];
+          _resolved
+            .pipeTo(
+              new WritableStream({
+                write(chunk) {
+                  chunks.push(chunk);
+                },
+                close() {
+                  resolve(Buffer.concat(chunks));
+                },
+                abort(reason) {
+                  reject(reason);
+                },
+              }),
+            )
+            .catch(reject);
+        });
+      } else if (typeof _resolved.pipe === "function") {
+        return new Promise<Buffer>((resolve, reject) => {
+          const chunks: Buffer[] = [];
+          _resolved
+            .on("data", (chunk: any) => {
+              chunks.push(chunk);
+            })
+            .on("end", () => {
+              resolve(Buffer.concat(chunks));
+            })
+            .on("error", reject);
+        });
+      }
       if (_resolved.constructor === Object) {
         return Buffer.from(JSON.stringify(_resolved));
       }
@@ -161,7 +195,32 @@ export async function readMultipartFormData(event: H3Event) {
  * ```
  */
 export async function readFormData(event: H3Event) {
-  return await event.request.formData();
+  return await toWebRequest(event).formData();
+}
+
+export function getRequestWebStream(
+  event: H3Event,
+): undefined | ReadableStream {
+  if (!PayloadMethods.includes(event.method)) {
+    return;
+  }
+  return (
+    event.web?.request?.body ||
+    (event._requestBody as ReadableStream) ||
+    new ReadableStream({
+      start: (controller) => {
+        event.node.req.on("data", (chunk) => {
+          controller.enqueue(chunk);
+        });
+        event.node.req.on("end", () => {
+          controller.close();
+        });
+        event.node.req.on("error", (err) => {
+          controller.error(err);
+        });
+      },
+    })
+  );
 }
 
 // --- Internal ---
diff --git a/src/utils/proxy.ts b/src/utils/proxy.ts
index 26a8268..a68b77f 100644
--- a/src/utils/proxy.ts
+++ b/src/utils/proxy.ts
@@ -3,7 +3,7 @@ import type { H3EventContext, RequestHeaders } from "../types";
 import { getRequestHeaders } from "./request";
 import { splitCookiesString } from "./cookie";
 import { sanitizeStatusMessage, sanitizeStatusCode } from "./sanitize";
-import { readRawBody } from "./body";
+import { getRequestWebStream, readRawBody } from "./body";
 
 export type Duplex = "half" | "full";
 
@@ -40,7 +40,7 @@ export async function proxyRequest(
   let duplex: Duplex | undefined;
   if (PayloadMethods.has(event.method)) {
     if (opts.streamRequest) {
-      body = event.rawBody;
+      body = getRequestWebStream(event);
       duplex = "half";
     } else {
       body = await readRawBody(event, false).catch(() => undefined);
diff --git a/src/utils/request.ts b/src/utils/request.ts
index 5ca2668..6408aab 100644
--- a/src/utils/request.ts
+++ b/src/utils/request.ts
@@ -3,6 +3,7 @@ import { createError } from "../error";
 import type { HTTPMethod, InferEventInput, RequestHeaders } from "../types";
 import type { H3Event } from "../event";
 import { validateData, ValidateFunction } from "./internal/validate";
+import { getRequestWebStream } from "./body";
 
 export function getQuery<
   T,
@@ -129,15 +130,10 @@ export function getRequestProtocol(
 }
 
 const DOUBLE_SLASH_RE = /[/\\]{2,}/g;
-
+/** @deprecated Use `event.path` instead */
 export function getRequestPath(event: H3Event): string {
-  const path = event._originalPath;
-  if (path.includes("?")) {
-    const [basePath, query] = path.split("?");
-    return basePath.replace(DOUBLE_SLASH_RE, "/") + "?" + query;
-  } else {
-    return path.replace(DOUBLE_SLASH_RE, "/");
-  }
+  const path = (event.node.req.url || "/").replace(DOUBLE_SLASH_RE, "/");
+  return path;
 }
 
 export function getRequestURL(
@@ -146,10 +142,26 @@ export function getRequestURL(
 ) {
   const host = getRequestHost(event, opts);
   const protocol = getRequestProtocol(event);
-  const path = getRequestPath(event);
+  const path = (event.node.req.originalUrl || event.path).replace(
+    /^[/\\]+/g,
+    "/",
+  );
   return new URL(path, `${protocol}://${host}`);
 }
 
+export function toWebRequest(event: H3Event) {
+  return (
+    event.web?.request ||
+    new Request(getRequestURL(event), {
+      // @ts-ignore Undici option
+      duplex: "half",
+      method: event.method,
+      headers: event.headers,
+      body: getRequestWebStream(event),
+    })
+  );
+}
+
 export function getRequestIP(
   event: H3Event,
   opts: {


Test patch:
diff --git a/test/event.test.ts b/test/event.test.ts
index 09317c0..998662a 100644
--- a/test/event.test.ts
+++ b/test/event.test.ts
@@ -1,6 +1,14 @@
 import supertest, { SuperTest, Test } from "supertest";
 import { describe, it, expect, beforeEach } from "vitest";
-import { createApp, App, toNodeListener, eventHandler } from "../src";
+import {
+  createApp,
+  App,
+  toNodeListener,
+  eventHandler,
+  readBody,
+  getRequestWebStream,
+  getRequestURL,
+} from "../src";
 
 describe("Event", () => {
   let app: App;
@@ -46,7 +54,7 @@ describe("Event", () => {
     app.use(
       "/",
       eventHandler((event) => {
-        return event.url;
+        return getRequestURL(event);
       }),
     );
     const result = await request.get("/hello");
@@ -57,8 +65,9 @@ describe("Event", () => {
     app.use(
       "/",
       eventHandler(async (event) => {
-        const bodyStream = event.rawBody as unknown as NodeJS.ReadableStream;
+        const bodyStream = getRequestWebStream(event);
         let bytes = 0;
+        // @ts-expect-error TODO: ReadableStream type is not async iterable!
         for await (const chunk of bodyStream) {
           bytes += chunk.length;
         }
@@ -77,13 +86,11 @@ describe("Event", () => {
     app.use(
       "/",
       eventHandler(async (event) => {
-        expect(event.request.method).toBe("POST");
-        expect(event.request.headers.get("x-test")).toBe("123");
+        expect(event.method).toBe("POST");
+        expect(event.headers.get("x-test")).toBe("123");
         // TODO: Find a workaround for Node.js 16
         if (!process.versions.node.startsWith("16")) {
-          expect(await event.request.text()).toMatchObject(
-            JSON.stringify({ hello: "world" }),
-          );
+          expect(await readBody(event)).toMatchObject({ hello: "world" });
         }
         return "200";
       }),
diff --git a/test/plain.test.ts b/test/plain.test.ts
index ab77fad..a58c998 100644
--- a/test/plain.test.ts
+++ b/test/plain.test.ts
@@ -5,6 +5,7 @@ import {
   toPlainHandler,
   PlainHandler,
   eventHandler,
+  readBody,
 } from "../src";
 
 describe("Plain handler", () => {
@@ -21,7 +22,7 @@ describe("Plain handler", () => {
       "/test",
       eventHandler(async (event) => {
         const body =
-          event.method === "POST" ? await event.request.text() : undefined;
+          event.method === "POST" ? await readBody(event) : undefined;
         event.node.res.statusCode = 201;
         event.node.res.statusMessage = "Created";
         event.node.res.setHeader("content-type", "application/json");
diff --git a/test/utils.test.ts b/test/utils.test.ts
index d0cbe09..bee4749 100644
--- a/test/utils.test.ts
+++ b/test/utils.test.ts
@@ -99,7 +99,7 @@ describe("", () => {
     const tests = [
       { path: "/foo", url: "http://127.0.0.1/foo" },
       { path: "//foo", url: "http://127.0.0.1/foo" },
-      { path: "//foo.com//bar", url: "http://127.0.0.1/foo.com/bar" },
+      { path: "//foo.com//bar", url: "http://127.0.0.1/foo.com//bar" },
       { path: "///foo", url: "http://127.0.0.1/foo" },
       { path: "\\foo", url: "http://127.0.0.1/foo" },
       { path: "\\\\foo", url: "http://127.0.0.1/foo" },
diff --git a/test/web.test.ts b/test/web.test.ts
index 92f6e36..2485021 100644
--- a/test/web.test.ts
+++ b/test/web.test.ts
@@ -1,5 +1,12 @@
 import { describe, it, expect, beforeEach } from "vitest";
-import { createApp, App, eventHandler, WebHandler, toWebHandler } from "../src";
+import {
+  createApp,
+  App,
+  eventHandler,
+  WebHandler,
+  toWebHandler,
+  readBody,
+} from "../src";
 
 describe("Web handler", () => {
   let app: App;
@@ -15,7 +22,7 @@ describe("Web handler", () => {
       "/test",
       eventHandler(async (event) => {
         const body =
-          event.method === "POST" ? await event.request.text() : undefined;
+          event.method === "POST" ? await readBody(event) : undefined;
         event.node.res.statusCode = 201;
         event.node.res.statusMessage = "Created";
         return {