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.

Sign up for GitHub

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

fix(cloudflare-pages): Expose Cloudflare Pages type parameters #3065

Merged
merged 1 commit into from
Jul 1, 2024
Merged

fix(cloudflare-pages): Expose Cloudflare Pages type parameters #3065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 100 additions & 71 deletions src/adapter/cloudflare-pages/handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,61 @@ import { getCookie } from '../../helper/cookie'
import { Hono } from '../../hono'
import { HTTPException } from '../../http-exception'
import type { EventContext } from './handler'
import { handle, handleMiddleware } from './handler'
import { handle, handleMiddleware, serveStatic } from './handler'

type Env = {
Bindings: {
TOKEN: string
eventContext: EventContext
}
}

function createEventContext(
context: Partial<EventContext<Env['Bindings']>>
): EventContext<Env['Bindings']> {
return {
data: {},
env: {
...context.env,
ASSETS: { fetch: vi.fn(), ...context.env?.ASSETS },
TOKEN: context.env?.TOKEN ?? 'HONOISCOOL',
},
functionPath: '_worker.js',
next: vi.fn(),
params: {},
passThroughOnException: vi.fn(),
request: new Request('http://localhost/api/foo'),
waitUntil: vi.fn(),
...context,
Comment on lines +13 to +29
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating an EventContext in each test was getting quite repetitive, so I added this function that turns a Partial<EventContext> into an EventContext with all the necessary properties.

}
}

describe('Adapter for Cloudflare Pages', () => {
it('Should return 200 response', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
ASSETS: { fetch },
TOKEN: 'HONOISCOOL',
}
const waitUntil = vi.fn()
const passThroughOnException = vi.fn()
const eventContext = createEventContext({
request,
env,
waitUntil,
passThroughOnException,
})
const app = new Hono<Env>()
const appFetchSpy = vi.spyOn(app, 'fetch')
app.get('/api/foo', (c) => {
const reqInEventContext = c.env.eventContext.request
return c.json({ TOKEN: c.env.TOKEN, requestURL: reqInEventContext.url })
Comment on lines -22 to -23
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, this was ensuring that the original eventContext was added to c.env

I replaced this by adding an assertion that vi.spyOn(app, 'fetch') was called with { ...env, eventContext }

return c.json({ TOKEN: c.env.TOKEN, requestURL: c.req.url })
})
const handler = handle(app)
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env })
const res = await handler(eventContext)
expect(appFetchSpy).toHaveBeenCalledWith(
request,
{ ...env, eventContext },
{ waitUntil, passThroughOnException }
)
expect(res.status).toBe(200)
expect(await res.json()).toEqual({
TOKEN: 'HONOISCOOL',
Expand All @@ -35,6 +66,7 @@ describe('Adapter for Cloudflare Pages', () => {

it('Should not use `basePath()` if path argument is not passed', async () => {
const request = new Request('http://localhost/api/error')
const eventContext = createEventContext({ request })
const app = new Hono().basePath('/api')

app.onError((e) => {
Expand All @@ -46,9 +78,7 @@ describe('Adapter for Cloudflare Pages', () => {

const handler = handle(app)
// It does throw the error if app is NOT "subApp"
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
expect(() => handler({ request })).toThrowError('Custom Error')
expect(() => handler(eventContext)).toThrowError('Custom Error')
})
})

Expand All @@ -59,9 +89,8 @@ describe('Middleware adapter for Cloudflare Pages', () => {
Cookie: 'my_cookie=1234',
},
})
const env = {
TOKEN: 'HONOISCOOL',
}
const next = vi.fn().mockResolvedValue(Response.json('From Cloudflare Pages'))
const eventContext = createEventContext({ request, next })
const handler = handleMiddleware(async (c, next) => {
const cookie = getCookie(c, 'my_cookie')

Expand All @@ -70,10 +99,7 @@ describe('Middleware adapter for Cloudflare Pages', () => {
return c.json({ cookie, response: await c.res.json() })
})

const next = vi.fn().mockResolvedValue(Response.json('From Cloudflare Pages'))
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env, next })
const res = await handler(eventContext)

expect(next).toHaveBeenCalled()

Expand All @@ -85,19 +111,15 @@ describe('Middleware adapter for Cloudflare Pages', () => {

it('Should return the middleware response when exceptions are handled', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware(async (c, next) => {
await next()

return c.json({ error: c.error?.message })
})

const next = vi.fn().mockRejectedValue(new Error('Error from next()'))
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env, next })
const eventContext = createEventContext({ request, next })
const res = await handler(eventContext)

expect(next).toHaveBeenCalled()

Expand All @@ -108,17 +130,13 @@ describe('Middleware adapter for Cloudflare Pages', () => {

it('Should return the middleware response if next() is not called', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware(async (c) => {
return c.json({ response: 'Skip Cloudflare Pages' })
})

const next = vi.fn()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env, next })
const eventContext = createEventContext({ request, next })
const res = await handler(eventContext)

expect(next).not.toHaveBeenCalled()

Expand All @@ -129,15 +147,11 @@ describe('Middleware adapter for Cloudflare Pages', () => {

it('Should return the Pages response if the middleware does not return a response', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware((c, next) => next())

const next = vi.fn().mockResolvedValue(Response.json('From Cloudflare Pages'))
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env, next })
const eventContext = createEventContext({ request, next })
const res = await handler(eventContext)

expect(next).toHaveBeenCalled()

Expand All @@ -146,18 +160,14 @@ describe('Middleware adapter for Cloudflare Pages', () => {

it('Should handle a HTTPException by returning error.getResponse()', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware(() => {
const res = new Response('Unauthorized', { status: 401 })
throw new HTTPException(401, { res })
})

const next = vi.fn()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env, next })
const eventContext = createEventContext({ request, next })
const res = await handler(eventContext)

expect(next).not.toHaveBeenCalled()

Expand All @@ -167,17 +177,13 @@ describe('Middleware adapter for Cloudflare Pages', () => {

it('Should handle an HTTPException thrown by next()', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware((c, next) => next())

const next = vi
.fn()
.mockRejectedValue(new HTTPException(401, { res: Response.json('Unauthorized') }))
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const res = await handler({ request, env, next })
const eventContext = createEventContext({ request, next })
const res = await handler(eventContext)

expect(next).toHaveBeenCalled()

Expand All @@ -186,58 +192,81 @@ describe('Middleware adapter for Cloudflare Pages', () => {

it('Should handle an Error thrown by next()', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware((c, next) => next())

const next = vi.fn().mockRejectedValue(new Error('Error from next()'))
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
await expect(handler({ request, env, next })).rejects.toThrowError('Error from next()')
const eventContext = createEventContext({ request, next })
await expect(handler(eventContext)).rejects.toThrowError('Error from next()')
expect(next).toHaveBeenCalled()
})

it('Should handle a non-Error thrown by next()', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware((c, next) => next())

const next = vi.fn().mockRejectedValue('Error from next()')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
await expect(handler({ request, env, next })).rejects.toThrowError('Error from next()')
const eventContext = createEventContext({ request, next })
await expect(handler(eventContext)).rejects.toThrowError('Error from next()')
expect(next).toHaveBeenCalled()
})

it('Should rethrow an Error', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware(() => {
throw new Error('Something went wrong')
})

const next = vi.fn()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
await expect(handler({ request, env, next })).rejects.toThrowError('Something went wrong')
const eventContext = createEventContext({ request, next })
await expect(handler(eventContext)).rejects.toThrowError('Something went wrong')
expect(next).not.toHaveBeenCalled()
})

it('Should rethrow non-Error exceptions', async () => {
const request = new Request('http://localhost/api/foo')
const env = {
TOKEN: 'HONOISCOOL',
}
const handler = handleMiddleware(() => Promise.reject('Something went wrong'))
const next = vi.fn()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
await expect(handler({ request, env, next })).rejects.toThrowError('Something went wrong')
const eventContext = createEventContext({ request, next })
await expect(handler(eventContext)).rejects.toThrowError('Something went wrong')
expect(next).not.toHaveBeenCalled()
})
})

describe('serveStatic()', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also added test coverage for serveStatic()

it('Should pass the raw request to ASSETS.fetch', async () => {
const assetsFetch = vi.fn().mockResolvedValue(new Response('foo.png'))
const request = new Request('http://localhost/foo.png')
const env = {
ASSETS: { fetch: assetsFetch },
TOKEN: 'HONOISCOOL',
}

const eventContext = createEventContext({ request, env })
const app = new Hono<Env>()
app.use(serveStatic())
const handler = handle(app)
const res = await handler(eventContext)

expect(assetsFetch).toHaveBeenCalledWith(request)
expect(res.status).toBe(200)
expect(await res.text()).toBe('foo.png')
})

it('Should respond with 404 if ASSETS.fetch returns a 404 response', async () => {
const assetsFetch = vi.fn().mockResolvedValue(new Response(null, { status: 404 }))
const request = new Request('http://localhost/foo.png')
const env = {
ASSETS: { fetch: assetsFetch },
TOKEN: 'HONOISCOOL',
}

const eventContext = createEventContext({ request, env })
const app = new Hono<Env>()
app.use(serveStatic())
const handler = handle(app)
const res = await handler(eventContext)

expect(assetsFetch).toHaveBeenCalledWith(request)
expect(res.status).toBe(404)
})
})
33 changes: 16 additions & 17 deletions src/adapter/cloudflare-pages/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Context } from '../../context'
import type { Hono } from '../../hono'
import { HTTPException } from '../../http-exception'
import type { Env, Input, MiddlewareHandler } from '../../types'
import type { BlankSchema, Env, Input, MiddlewareHandler, Schema } from '../../types'

// Ref: https://github.com/cloudflare/workerd/blob/main/types/defines/pages.d.ts

Expand All @@ -27,26 +27,25 @@ declare type PagesFunction<
Data extends Record<string, unknown> = Record<string, unknown>
> = (context: EventContext<Env, Params, Data>) => Response | Promise<Response>

interface HandleInterface {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(app: Hono<any, any, any>): (eventContext: EventContext) => Response | Promise<Response>
}

export const handle: HandleInterface = (app: Hono) => (eventContext: EventContext) => {
return app.fetch(
eventContext.request,
{ ...eventContext.env, eventContext },
{
waitUntil: eventContext.waitUntil,
passThroughOnException: eventContext.passThroughOnException,
}
)
}
export const handle =
<E extends Env = Env, S extends Schema = BlankSchema, BasePath extends string = '/'>(
app: Hono<E, S, BasePath>
): PagesFunction<E['Bindings']> =>
(eventContext) => {
return app.fetch(
eventContext.request,
{ ...eventContext.env, eventContext },
{
waitUntil: eventContext.waitUntil,
passThroughOnException: eventContext.passThroughOnException,
}
)
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function handleMiddleware<E extends Env = any, P extends string = any, I extends Input = {}>(
middleware: MiddlewareHandler<E, P, I>
): PagesFunction {
): PagesFunction<E['Bindings']> {
return async (executionCtx) => {
const context = new Context(executionCtx.request, {
env: executionCtx.env,
Expand Down