fix: enable to return Response like object (#2085)

* fix: enable to return Response like object This improves behavior, especially in cases where Response is being overwritten on a node-server. * chore: denoify
honojs · Jan 27, 2024 · 8c62c64 · 8c62c64
1 parent b23cf8d
commit 8c62c64
Show file tree

Hide file tree

Showing 3 changed files with 220 additions and 71 deletions.
diff --git a/deno_dist/hono-base.ts b/deno_dist/hono-base.ts
@@ -315,14 +315,14 @@ class Hono<
         return this.handleError(err, c)
       }
 
-      if (res instanceof Response) return res
-
-      return res
-        .then(
-          (resolved: Response | undefined) =>
-            resolved || (c.finalized ? c.res : this.notFoundHandler(c))
-        )
-        .catch((err: Error) => this.handleError(err, c))
+      return res instanceof Promise
+        ? res
+            .then(
+              (resolved: Response | undefined) =>
+                resolved || (c.finalized ? c.res : this.notFoundHandler(c))
+            )
+            .catch((err: Error) => this.handleError(err, c))
+        : res
     }
 
     const composed = compose<Context>(matchResult[0], this.errorHandler, this.notFoundHandler)

diff --git a/src/hono-base.ts b/src/hono-base.ts
@@ -315,14 +315,14 @@ class Hono<
         return this.handleError(err, c)
       }
 
-      if (res instanceof Response) return res
-
-      return res
-        .then(
-          (resolved: Response | undefined) =>
-            resolved || (c.finalized ? c.res : this.notFoundHandler(c))
-        )
-        .catch((err: Error) => this.handleError(err, c))
+      return res instanceof Promise
+        ? res
+            .then(
+              (resolved: Response | undefined) =>
+                resolved || (c.finalized ? c.res : this.notFoundHandler(c))
+            )
+            .catch((err: Error) => this.handleError(err, c))
+        : res
     }
 
     const composed = compose<Context>(matchResult[0], this.errorHandler, this.notFoundHandler)

diff --git a/src/hono.test.ts b/src/hono.test.ts
@@ -20,73 +20,222 @@ function throwExpression(errorMessage: string): never {
 }
 
 describe('GET Request', () => {
-  const app = new Hono()
+  describe('without middleware', () => {
+    // In other words, this is a test for cases that do not use `compose()`
 
-  app.get('/hello', async () => {
-    return new Response('hello', {
-      status: 200,
-      statusText: 'Hono is OK',
+    const app = new Hono()
+
+    app.get('/hello', async () => {
+      return new Response('hello', {
+        status: 200,
+        statusText: 'Hono is OK',
+      })
     })
-  })
 
-  app.get('/hello-with-shortcuts', (c) => {
-    c.header('X-Custom', 'This is Hono')
-    c.status(201)
-    return c.html('<h1>Hono!!!</h1>')
-  })
+    app.get('/hello-with-shortcuts', (c) => {
+      c.header('X-Custom', 'This is Hono')
+      c.status(201)
+      return c.html('<h1>Hono!!!</h1>')
+    })
 
-  app.get('/hello-env', (c) => {
-    return c.json(c.env)
-  })
+    app.get('/hello-env', (c) => {
+      return c.json(c.env)
+    })
 
-  it('GET http://localhost/hello is ok', async () => {
-    const res = await app.request('http://localhost/hello')
-    expect(res).not.toBeNull()
-    expect(res.status).toBe(200)
-    expect(res.statusText).toBe('Hono is OK')
-    expect(await res.text()).toBe('hello')
-  })
+    app.get(
+      '/proxy-object',
+      () =>
+        new Proxy(new Response('proxy'), {
+          get(target, prop: keyof Response) {
+            return target[prop]
+          },
+        })
+    )
 
-  it('GET httphello is ng', async () => {
-    const res = await app.request('httphello')
-    expect(res.status).toBe(404)
-  })
+    app.get(
+      '/async-proxy-object',
+      async () =>
+        new Proxy(new Response('proxy'), {
+          get(target, prop: keyof Response) {
+            return target[prop]
+          },
+        })
+    )
 
-  it('GET /hello is ok', async () => {
-    const res = await app.request('/hello')
-    expect(res).not.toBeNull()
-    expect(res.status).toBe(200)
-    expect(res.statusText).toBe('Hono is OK')
-    expect(await res.text()).toBe('hello')
-  })
+    it('GET http://localhost/hello is ok', async () => {
+      const res = await app.request('http://localhost/hello')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(res.statusText).toBe('Hono is OK')
+      expect(await res.text()).toBe('hello')
+    })
 
-  it('GET hello is ok', async () => {
-    const res = await app.request('hello')
-    expect(res).not.toBeNull()
-    expect(res.status).toBe(200)
-    expect(res.statusText).toBe('Hono is OK')
-    expect(await res.text()).toBe('hello')
-  })
+    it('GET httphello is ng', async () => {
+      const res = await app.request('httphello')
+      expect(res.status).toBe(404)
+    })
 
-  it('GET /hello-with-shortcuts is ok', async () => {
-    const res = await app.request('http://localhost/hello-with-shortcuts')
-    expect(res).not.toBeNull()
-    expect(res.status).toBe(201)
-    expect(res.headers.get('X-Custom')).toBe('This is Hono')
-    expect(res.headers.get('Content-Type')).toMatch(/text\/html/)
-    expect(await res.text()).toBe('<h1>Hono!!!</h1>')
-  })
+    it('GET /hello is ok', async () => {
+      const res = await app.request('/hello')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(res.statusText).toBe('Hono is OK')
+      expect(await res.text()).toBe('hello')
+    })
 
-  it('GET / is not found', async () => {
-    const res = await app.request('http://localhost/')
-    expect(res).not.toBeNull()
-    expect(res.status).toBe(404)
+    it('GET hello is ok', async () => {
+      const res = await app.request('hello')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(res.statusText).toBe('Hono is OK')
+      expect(await res.text()).toBe('hello')
+    })
+
+    it('GET /hello-with-shortcuts is ok', async () => {
+      const res = await app.request('http://localhost/hello-with-shortcuts')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(201)
+      expect(res.headers.get('X-Custom')).toBe('This is Hono')
+      expect(res.headers.get('Content-Type')).toMatch(/text\/html/)
+      expect(await res.text()).toBe('<h1>Hono!!!</h1>')
+    })
+
+    it('GET / is not found', async () => {
+      const res = await app.request('http://localhost/')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(404)
+    })
+
+    it('GET /hello-env is ok', async () => {
+      const res = await app.request('/hello-env', undefined, { HELLO: 'world' })
+      expect(res.status).toBe(200)
+      expect(await res.json()).toEqual({ HELLO: 'world' })
+    })
+
+    it('GET /proxy-object is ok', async () => {
+      const res = await app.request('/proxy-object')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('proxy')
+    })
+
+    it('GET /async-proxy-object is ok', async () => {
+      const res = await app.request('/proxy-object')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('proxy')
+    })
   })
 
-  it('GET /hello-env is ok', async () => {
-    const res = await app.request('/hello-env', undefined, { HELLO: 'world' })
-    expect(res.status).toBe(200)
-    expect(await res.json()).toEqual({ HELLO: 'world' })
+  describe('with middleware', () => {
+    // when using `compose()`
+
+    const app = new Hono()
+
+    app.use('*', async (ctx, next) => {
+      await next()
+    })
+
+    app.get('/hello', async () => {
+      return new Response('hello', {
+        status: 200,
+        statusText: 'Hono is OK',
+      })
+    })
+
+    app.get('/hello-with-shortcuts', (c) => {
+      c.header('X-Custom', 'This is Hono')
+      c.status(201)
+      return c.html('<h1>Hono!!!</h1>')
+    })
+
+    app.get('/hello-env', (c) => {
+      return c.json(c.env)
+    })
+
+    app.get(
+      '/proxy-object',
+      () =>
+        new Proxy(new Response('proxy'), {
+          get(target, prop: keyof Response) {
+            return target[prop]
+          },
+        })
+    )
+
+    app.get(
+      '/async-proxy-object',
+      async () =>
+        new Proxy(new Response('proxy'), {
+          get(target, prop: keyof Response) {
+            return target[prop]
+          },
+        })
+    )
+
+    it('GET http://localhost/hello is ok', async () => {
+      const res = await app.request('http://localhost/hello')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(res.statusText).toBe('Hono is OK')
+      expect(await res.text()).toBe('hello')
+    })
+
+    it('GET httphello is ng', async () => {
+      const res = await app.request('httphello')
+      expect(res.status).toBe(404)
+    })
+
+    it('GET /hello is ok', async () => {
+      const res = await app.request('/hello')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(res.statusText).toBe('Hono is OK')
+      expect(await res.text()).toBe('hello')
+    })
+
+    it('GET hello is ok', async () => {
+      const res = await app.request('hello')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(res.statusText).toBe('Hono is OK')
+      expect(await res.text()).toBe('hello')
+    })
+
+    it('GET /hello-with-shortcuts is ok', async () => {
+      const res = await app.request('http://localhost/hello-with-shortcuts')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(201)
+      expect(res.headers.get('X-Custom')).toBe('This is Hono')
+      expect(res.headers.get('Content-Type')).toMatch(/text\/html/)
+      expect(await res.text()).toBe('<h1>Hono!!!</h1>')
+    })
+
+    it('GET / is not found', async () => {
+      const res = await app.request('http://localhost/')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(404)
+    })
+
+    it('GET /hello-env is ok', async () => {
+      const res = await app.request('/hello-env', undefined, { HELLO: 'world' })
+      expect(res.status).toBe(200)
+      expect(await res.json()).toEqual({ HELLO: 'world' })
+    })
+
+    it('GET /proxy-object is ok', async () => {
+      const res = await app.request('/proxy-object')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('proxy')
+    })
+
+    it('GET /async-proxy-object is ok', async () => {
+      const res = await app.request('/proxy-object')
+      expect(res).not.toBeNull()
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('proxy')
+    })
   })
 })