fix(proxy): remove spread operator from Request and Response classes

[BarryThePenguin]

Removes usage of the spread operator (...) on Request and Response classes

Follow-up from #3927

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.29%. Comparing base (0c405b9) to head (b1af554).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3928      +/-   ##
==========================================
- Coverage   91.32%   91.29%   -0.04%     
==========================================
  Files         168      168              
  Lines       10689    10774      +85     
  Branches     3056     3101      +45     
==========================================
+ Hits         9762     9836      +74     
- Misses        926      937      +11     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[usualoma]

Hi @BarryThePenguin. Thank you.

Please do not modify the proxyInit object received as an argument, as follows.

diff --git a/src/helper/proxy/index.ts b/src/helper/proxy/index.ts
index 4c010a2d..93a7543b 100644
--- a/src/helper/proxy/index.ts
+++ b/src/helper/proxy/index.ts
@@ -49,13 +49,13 @@ const buildRequestInitFromRequest = (
   }
 }
 
-const preprocessRequestInit = (requestInit: RequestInit): RequestInit => {
+const preprocessRequestInit = (requestInit: Readonly<ProxyRequestInit>): RequestInit => {
   if (
     !requestInit.headers ||
     Array.isArray(requestInit.headers) ||
     requestInit.headers instanceof Headers
   ) {
-    return requestInit
+    return requestInit as RequestInit
   }
 
   const headers = new Headers()
@@ -67,8 +67,10 @@ const preprocessRequestInit = (requestInit: RequestInit): RequestInit => {
       headers.set(key, value)
     }
   }
-  requestInit.headers = headers
-  return requestInit
+  return {
+    ...requestInit,
+    headers,
+  }
 }
 
 /**
@@ -111,7 +113,7 @@ const preprocessRequestInit = (requestInit: RequestInit): RequestInit => {
 export const proxy: ProxyFetch = async (input, proxyInit = {}) => {
   const req = new Request(input, {
     ...buildRequestInitFromRequest(proxyInit.raw),
-    ...preprocessRequestInit(proxyInit as RequestInit),
+    ...preprocessRequestInit(proxyInit),
   })
   req.headers.delete('accept-encoding')
 

Also, if possible, since the changes to src/helper/proxy/index.ts are different from the following, it would be helpful if you could include only one purpose in one PR.

Removes usage of the spread operator (...) on Request and Response classes

[BarryThePenguin]

Also, if possible, since the changes to src/helper/proxy/index.ts are different from the following, it would be helpful if you could include only one purpose in one PR.

I was trying to avoid using the spread operator on proxyInit

hono/src/helper/proxy/index.ts

Line 113 in 6ae0213

const { raw, ...requestInit } = proxyInit ?? {}

As the current API for proxy makes it very easy to pass Request as proxyInit, for example

app.get('/proxy/:path', (c) => {
  return proxy(`http://${originServer}/${c.req.param('path')}`, c.req.raw)
})

Though I'm not sure if this is intentional or not. I'll revert the changes to src/helper/proxy/index.ts

[usualoma]

Hi @BarryThePenguin. Thank you for getting back to me.

I see, as you say, it would be better if the following code (although it is not the recommended way to use it) processed c.req.raw as RequestInit. Or, if it is not processed, it would be better if it caused an error.

app.get(‘/proxy/:path’, (c) => {
  return proxy(`http://${originServer}/${c.req.param(‘path’)}`, c.req.raw)
})

Let's discuss it in another PR.

[yusukebe]

LGTM!

[yusukebe]

@BarryThePenguin

Thank you. Looks good. I'll merge this now.