ronantakizawa/codereview-bench huggingface dataset:
Repository: honojs/hono
File: deno_dist/middleware/timing/index.ts (TypeScript)

Code before:
import type { Context } from '../../context.ts'
import type { MiddlewareHandler } from '../../types.ts'
import '../../context.ts'

declare module '../../context.ts' {
  interface ContextVariableMap {
    metric?: {
      headers: string[]
      timers: Map<string, Timer>
    }
  }
}

interface Timer {
  description?: string
  start: number
}

interface Config {
  total: boolean
  enabled: boolean | ((c: Context) => boolean)
  totalDescription: string
  autoEnd: boolean
  crossOrigin: boolean | string
}

const getTime = () => {
  try {
    return performance.now()
  } catch {}
  return Date.now()
}

export const timing = (config?: Partial<Config>): MiddlewareHandler => {
  const options: Config = {
    ...{
      total: true,
      enabled: true,
      totalDescription: 'Total Response Time',
      autoEnd: true,
      crossOrigin: false,
    },
    ...config,
  }

Review comment (suggestion):
It's better to be `TimingOptions` like a`CORSOptions`.

Code after:
import type { Context } from '../../context.ts'
import type { MiddlewareHandler } from '../../types.ts'
import '../../context.ts'

declare module '../../context.ts' {
  interface ContextVariableMap {
    metric?: {
      headers: string[]
      timers: Map<string, Timer>
    }
  }
}

interface Timer {
  description?: string
  start: number
}

interface TimingOptions {
  total: boolean
  enabled: boolean | ((c: Context) => boolean)
  totalDescription: string
  autoEnd: boolean
  crossOrigin: boolean | string
}

const getTime = () => {
  try {
    return performance.now()
  } catch {}
  return Date.now()
}

export const timing = (config?: Partial<TimingOptions>): MiddlewareHandler => {
  const options: TimingOptions = {
    ...{
      total: true,
      enabled: true,
      totalDescription: 'Total Response Time',
      autoEnd: true,
      crossOrigin: false,
    },
    ...config,
  }
Repository: honojs/hono
File: deno_dist/middleware/timing/index.ts (TypeScript)

Code before:
      timers: Map<string, Timer>
    }
  }
}

interface Timer {
  description?: string
  start: number
}

interface Config {
  total: boolean
  enabled: boolean | ((c: Context) => boolean)
  totalDescription: string
  autoEnd: boolean
  crossOrigin: boolean | string
}

const getTime = () => {
  try {
    return performance.now()
  } catch {}
  return Date.now()
}

export const timing = (config?: Partial<Config>): MiddlewareHandler => {
  const options: Config = {
    ...{
      total: true,
      enabled: true,
      totalDescription: 'Total Response Time',
      autoEnd: true,
      crossOrigin: false,
    },
    ...config,
  }
  return async (c, next) => {
    const headers: string[] = []
    const timers = new Map<string, Timer>()
    c.set('metric', { headers, timers })

    if (options.total) {
      startTime(c, 'total', options.totalDescription)
    }
    await next()

    if (options.total) {
      endTime(c, 'total')
    }

    if (options.autoEnd) {

Review comment (suggestion):
It also should be `options?: Partial<TimingOptions>`

Code after:
      timers: Map<string, Timer>
    }
  }
}

interface Timer {
  description?: string
  start: number
}

interface TimingOptions {
  total: boolean
  enabled: boolean | ((c: Context) => boolean)
  totalDescription: string
  autoEnd: boolean
  crossOrigin: boolean | string
}

const getTime = () => {
  try {
    return performance.now()
  } catch {}
  return Date.now()
}

export const timing = (config?: Partial<TimingOptions>): MiddlewareHandler => {
  const options: TimingOptions = {
    ...{
      total: true,
      enabled: true,
      totalDescription: 'Total Response Time',
      autoEnd: true,
      crossOrigin: false,
    },
    ...config,
  }
  return async (c, next) => {
    const headers: string[] = []
    const timers = new Map<string, Timer>()
    c.set('metric', { headers, timers })

    if (options.total) {
      startTime(c, 'total', options.totalDescription)
    }
    await next()

    if (options.total) {
      endTime(c, 'total')
    }

    if (options.autoEnd) {
Repository: honojs/hono
File: src/request.ts (TypeScript)

Code before:
   * @example
   *
   * import { getCookie } from 'hono/cookie'
   * // ...
   * app.get('/', (c) => c.json(getCookie(c)))
   */
  cookie(): Cookie

  cookie(key?: string) {
    const cookie = this.raw.headers.get('Cookie')
    if (!cookie) return
    const obj = parse(cookie)
    if (key) {
      const value = obj[key]
      return value
    } else {
      return obj
    }
  }

  async parseBody<T extends BodyData = BodyData>(): Promise<T> {
    return await parseBody(this)
  }

  private cachedBody = (key: keyof BodyCache) => {
    const cachedBody = this.bodyCache[key]
    if (cachedBody) return cachedBody
    this.bodyCache[key] = this.raw[key]()
    return this.bodyCache[key]
  }

  json<T = any>(): Promise<T> {
    return this.cachedBody('json')
  }

  text(): Promise<string> {
    return this.cachedBody('text')
  }

  arrayBuffer(): Promise<ArrayBuffer> {
    return this.cachedBody('arrayBuffer')
  }

  blob(): Promise<Blob> {
    return this.cachedBody('blob')
  }

  formData(): Promise<FormData> {
    return this.cachedBody('formData')
  }


Review comment (performance):
I like this a little better for perf since it reduces calls to getters. But maybe less clear?
```ts
    const { bodyCache, raw } = this
    const cachedBody = bodyCache[key]
    if (cachedBody) return cachedBody
    return (bodyCache[key] = raw[key]())
```

Code after:
   * @example
   *
   * import { getCookie } from 'hono/cookie'
   * // ...
   * app.get('/', (c) => c.json(getCookie(c)))
   */
  cookie(): Cookie

  cookie(key?: string) {
    const cookie = this.raw.headers.get('Cookie')
    if (!cookie) return
    const obj = parse(cookie)
    if (key) {
      const value = obj[key]
      return value
    } else {
      return obj
    }
  }

  async parseBody<T extends BodyData = BodyData>(): Promise<T> {
    return await parseBody(this)
  }

  private cachedBody = (key: keyof BodyCache) => {
    const { bodyCache, raw } = this
    const cachedBody = bodyCache[key]
    if (cachedBody) return cachedBody
    return (bodyCache[key] = raw[key]())
  }

  json<T = any>(): Promise<T> {
    return this.cachedBody('json')
  }

  text(): Promise<string> {
    return this.cachedBody('text')
  }

  arrayBuffer(): Promise<ArrayBuffer> {
    return this.cachedBody('arrayBuffer')
  }

  blob(): Promise<Blob> {
    return this.cachedBody('blob')
  }

  formData(): Promise<FormData> {
    return this.cachedBody('formData')
  }
Repository: honojs/hono
File: src/middleware/secure-headers/index.test.ts (TypeScript)

Code before:
import { Hono } from '../../hono'
import { poweredBy } from '../powered-by'
import { secureHeaders } from '.'

describe('Secure Headers Middleware', () => {
  it('all headers enabled', async () => {
    const app = new Hono()
    app.use('*', secureHeaders())
    app.get('/test', async (ctx) => {
      return ctx.text('test')
    })

    const res = await app.request('/test')
    expect(res).not.toBeNull()
    expect(res.status).toBe(200)
    expect(res.headers.get('X-Frame-Options')).toEqual('SAMEORIGIN')
    expect(res.headers.get('Strict-Transport-Security')).toEqual(
      'max-age=15552000; includeSubDomains'
    )
    expect(res.headers.get('X-Download-Options')).toEqual('noopen')
    expect(res.headers.get('X-XSS-Protection')).toEqual('0')
    expect(res.headers.get('X-Powered-By')).toBeNull()
    expect(res.headers.get('X-DNS-Prefetch-Control')).toEqual('off')
    expect(res.headers.get('X-Content-Type-Options')).toEqual('nosniff')
    expect(res.headers.get('Referrer-Policy')).toEqual('no-referrer')
    expect(res.headers.get('X-Permitted-Cross-Domain-Policies')).toEqual('none')
    expect(res.headers.get('Cross-Origin-Resource-Policy')).toEqual('same-origin')

Review comment (suggestion):
You can add the follow line on the top to fix lint errors:

```ts
/* eslint-disable quotes */
```

This allow using double quotes.

Code after:
/* eslint-disable quotes */
import { Hono } from '../../hono'
import { poweredBy } from '../powered-by'
import { secureHeaders } from '.'

describe('Secure Headers Middleware', () => {
  it('default middleware', async () => {
    const app = new Hono()
    app.use('*', secureHeaders())
    app.get('/test', async (ctx) => {
      return ctx.text('test')
    })

    const res = await app.request('/test')
    expect(res).not.toBeNull()
    expect(res.status).toBe(200)
    expect(res.headers.get('X-Frame-Options')).toEqual('SAMEORIGIN')
    expect(res.headers.get('Strict-Transport-Security')).toEqual(
      'max-age=15552000; includeSubDomains'
    )
    expect(res.headers.get('X-Download-Options')).toEqual('noopen')
    expect(res.headers.get('X-XSS-Protection')).toEqual('0')
    expect(res.headers.get('X-Powered-By')).toBeNull()
    expect(res.headers.get('X-DNS-Prefetch-Control')).toEqual('off')
    expect(res.headers.get('X-Content-Type-Options')).toEqual('nosniff')
    expect(res.headers.get('Referrer-Policy')).toEqual('no-referrer')
    expect(res.headers.get('X-Permitted-Cross-Domain-Policies')).toEqual('none')
Repository: honojs/hono
File: src/helper/cookie/index.ts (TypeScript)

Code before:
  const obj = await parseSigned(cookie, secret)
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  return obj as any
}

export const setCookie = (c: Context, name: string, value: string, opt?: CookieOptions): void => {
  const cookie = serialize(name, value, opt)
  c.header('set-cookie', cookie, { append: true })
}

export const setSignedCookie = async (
  c: Context,
  name: string,
  value: string,
  secret: string | BufferSource,
  opt?: CookieOptions
): Promise<void> => {
  const cookie = await serializeSigned(name, value, secret, opt)
  c.header('set-cookie', cookie, { append: true })
}

export const deleteCookie = (c: Context, name: string, opt?: CookieOptions): void => {
  setCookie(c, name, '', { ...opt, maxAge: 0 })
}

export const getCookieValue = (cookie: string, key: string): string => {
  const obj = parse(cookie)
  return obj[key] || ''
}


Review comment (refactor):
Since `getCookieValue()` is only used in `src/client/client.test.ts`, you can define it in `src/client/client.test.ts`. You should not include it in the helper to add unnecessary APIs or keep it in size.

Code after:
import type { Context } from '../../context'
import { parse, parseSigned, serialize, serializeSigned } from '../../utils/cookie'
import type { CookieOptions, Cookie, SignedCookie } from '../../utils/cookie'

interface GetCookie {
  (c: Context, key: string): string | undefined
  (c: Context): Cookie
}

interface GetSignedCookie {
  (c: Context, secret: string | BufferSource, key: string): Promise<string | undefined | false>
  (c: Context, secret: string): Promise<SignedCookie>
}

export const getCookie: GetCookie = (c, key?) => {
  const cookie = c.req.raw.headers.get('Cookie')
  if (typeof key === 'string') {
    if (!cookie) return undefined
    const obj = parse(cookie)
    return obj[key]
  }
  if (!cookie) return {}
  const obj = parse(cookie)
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  return obj as any
}

export const getSignedCookie: GetSignedCookie = async (c, secret, key?) => {
  const cookie = c.req.raw.headers.get('Cookie')
  if (typeof key === 'string') {
    if (!cookie) return undefined
    const obj = await parseSigned(cookie, secret, key)
    return obj[key]
  }
  if (!cookie) return {}
  const obj = await parseSigned(cookie, secret)
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  return obj as any
}

export const setCookie = (c: Context, name: string, value: string, opt?: CookieOptions): void => {
  const cookie = serialize(name, value, opt)
  c.header('set-cookie', cookie, { append: true })
}

export const setSignedCookie = async (
  c: Context,
  name: string,
  value: string,
  secret: string | BufferSource,
  opt?: CookieOptions
): Promise<void> => {
  const cookie = await serializeSigned(name, value, secret, opt)
  c.header('set-cookie', cookie, { append: true })
}

export const deleteCookie = (c: Context, name: string, opt?: CookieOptions): void => {
  setCookie(c, name, '', { ...opt, maxAge: 0 })
}
Repository: honojs/hono
File: src/hono.ts (TypeScript)

Code before:
import { HonoBase } from './hono-base'
import { RegExpRouter } from './router/reg-exp-router'
import { SmartRouter } from './router/smart-router'
import { TrieRouter } from './router/trie-router'
import type { Env, Schema } from './types'

export class Hono<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> extends HonoBase<E, S, BasePath> {
  constructor(
    init: Exclude<ConstructorParameters<typeof HonoBase>[0], 'basePath'> & {
      basePath?: BasePath
    } = {}
  ) {
    super(init)
    this.router =
      init.router ??
      new SmartRouter({
        routers: [new RegExpRouter(), new TrieRouter()],
      })
  }
}


Review comment (suggestion):
I've been wondering, why not just make the constructor params into a new type? since it's the same one with the other preset

Code after:
import { HonoBase } from './hono-base'
import type { HonoOptions } from './hono-base'
import { RegExpRouter } from './router/reg-exp-router'
import { SmartRouter } from './router/smart-router'
import { TrieRouter } from './router/trie-router'
import type { Env, Schema } from './types'

export class Hono<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> extends HonoBase<E, S, BasePath> {
  constructor(options: HonoOptions<E, BasePath> = {}) {
    super(options)
    this.router =
      options.router ??
      new SmartRouter({
        routers: [new RegExpRouter(), new TrieRouter()],
      })
  }
}
Repository: honojs/hono
File: src/types.ts (TypeScript)

Code before:
////////////////////////////////////////

export type InputToDataByTarget<
  T extends Input['out'],
  Target extends keyof ValidationTargets
> = T extends {
  [K in Target]: infer R
}
  ? R
  : never

export type RemoveQuestion<T> = T extends `${infer R}?` ? R : T

export type UndefinedIfHavingQuestion<T> = T extends `${infer _}?` ? string | undefined : string

////////////////////////////////////////
//////                            //////
//////         Utilities          //////
//////                            //////
////////////////////////////////////////

export type ExtractSchema<T> = UnionToIntersection<
  T extends Hono<infer _, infer S, any> ? S : never
>

export type RequiredKeysOf<BaseType extends object> = Exclude<
  {
    [Key in keyof BaseType]: BaseType extends Record<Key, BaseType[Key]> ? Key : never
  }[keyof BaseType],
  undefined
>

export type HasRequiredKeys<BaseType extends object> = RequiredKeysOf<BaseType> extends never
  ? false
  : true

////////////////////////////////////////
//////                            //////
//////         FetchEvent         //////
//////                            //////
////////////////////////////////////////

export abstract class FetchEventLike {
  abstract readonly request: Request
  abstract respondWith(promise: Response | Promise<Response>): void
  abstract passThroughOnException(): void
  abstract waitUntil(promise: Promise<void>): void
}


Review comment (suggestion):
Please place `RequiredKeysOf` and `HasRequiredKeys` in `src/utils/types.ts` as they may be used in other places.

Code after:
////////////////////////////////////////

export type InputToDataByTarget<
  T extends Input['out'],
  Target extends keyof ValidationTargets
> = T extends {
  [K in Target]: infer R
}
  ? R
  : never

export type RemoveQuestion<T> = T extends `${infer R}?` ? R : T

export type UndefinedIfHavingQuestion<T> = T extends `${infer _}?` ? string | undefined : string

////////////////////////////////////////
//////                            //////
//////         Utilities          //////
//////                            //////
////////////////////////////////////////

export type ExtractSchema<T> = UnionToIntersection<
  T extends Hono<infer _, infer S, any> ? S : never
>

////////////////////////////////////////
//////                            //////
//////         FetchEvent         //////
//////                            //////
////////////////////////////////////////

export abstract class FetchEventLike {
  abstract readonly request: Request
  abstract respondWith(promise: Response | Promise<Response>): void
  abstract passThroughOnException(): void
  abstract waitUntil(promise: Promise<void>): void
}
Repository: honojs/hono
File: deno_dist/helper/dev/index.ts (TypeScript)

Code before:
import type { Hono } from '../../hono.ts'
import type { Env, RouterRoute } from '../../types.ts'

interface ShowRoutesOptions {
  verbose?: boolean
}

interface RouteData {
  path: string
  method: string
  name: string
  isMiddleware: boolean
}

const isMiddleware = (handler: Function) => handler.length > 1
const handlerName = (handler: Function) => {
  return handler.name || (isMiddleware(handler) ? '[middleware]' : '[handler]')
}

export const inspectRoutes = <TEnv extends Env>(hono: Hono<TEnv>): RouteData[] => {
  return hono.routes.map(({ path, method, handler }: RouterRoute) => ({
    path,
    method,
    name: handlerName(handler),
    isMiddleware: isMiddleware(handler),
  }))
}

export const showRoutes = <TEnv extends Env>(hono: Hono<TEnv>, opts?: ShowRoutesOptions) => {
  const routeData: Record<string, RouteData[]> = {}
  let maxMethodLength = 0
  let maxPathLength = 0

  inspectRoutes(hono)
    .filter(({ isMiddleware }) => opts?.verbose || !isMiddleware)
    .map((route) => {
      const key = `${route.method}-${route.path}`
      ;(routeData[key] ||= []).push(route)
      if (routeData[key].length > 1) {
        return
      }
      maxMethodLength = Math.max(maxMethodLength, route.method.length)
      maxPathLength = Math.max(maxPathLength, route.path.length)
      return { method: route.method, path: route.path, routes: routeData[key] }
    })

Review comment (suggestion):
We use `E` instead of `TEnv` in the same cases. So it would be better to use `E`.

https://github.com/honojs/hono/blob/5e0ef3048b5add5dadbbb4e936254b71058351b2/src/hono-base.ts#L53

Code after:
import type { Hono } from '../../hono.ts'
import type { Env, RouterRoute } from '../../types.ts'

interface ShowRoutesOptions {
  verbose?: boolean
}

interface RouteData {
  path: string
  method: string
  name: string
  isMiddleware: boolean
}

const isMiddleware = (handler: Function) => handler.length > 1
const handlerName = (handler: Function) => {
  return handler.name || (isMiddleware(handler) ? '[middleware]' : '[handler]')
}

export const inspectRoutes = <E extends Env>(hono: Hono<E>): RouteData[] => {
  return hono.routes.map(({ path, method, handler }: RouterRoute) => ({
    path,
    method,
    name: handlerName(handler),
    isMiddleware: isMiddleware(handler),
  }))
}

export const showRoutes = <E extends Env>(hono: Hono<E>, opts?: ShowRoutesOptions) => {
  const routeData: Record<string, RouteData[]> = {}
  let maxMethodLength = 0
  let maxPathLength = 0

  inspectRoutes(hono)
    .filter(({ isMiddleware }) => opts?.verbose || !isMiddleware)
    .map((route) => {
      const key = `${route.method}-${route.path}`
      ;(routeData[key] ||= []).push(route)
      if (routeData[key].length > 1) {
        return
      }
      maxMethodLength = Math.max(maxMethodLength, route.method.length)
      maxPathLength = Math.max(maxPathLength, route.path.length)
      return { method: route.method, path: route.path, routes: routeData[key] }
    })
Repository: honojs/hono
File: benchmarks/routers/src/bench-includes-init.mts (TypeScript)

Code before:
import MedleyRouter from '@medley/router'
import type { HTTPMethod } from 'find-my-way'
import findMyWay from 'find-my-way'
import KoaRouter from 'koa-tree-router'
import { run, bench, group } from 'mitata'
import TrekRouter from 'trek-router'
import { LinearRouter } from '../../../src/router/linear-router/index.ts'
import {
  RegExpRouter,
  PreparedRegExpRouter,
  buildInitParams,
} from '../../../src/router/reg-exp-router/index.ts'
import { TrieRouter } from '../../../src/router/trie-router/index.ts'
import type { Route } from './tool.mts'
import { routes } from './tool.mts'

const preparedParams = buildInitParams({
  routes
})

const benchRoutes: (Route & { name: string })[] = [
  {
    name: 'short static',
    method: 'GET',
    path: '/user',
  },
  {
    name: 'static with same radix',
    method: 'GET',
    path: '/user/comments',
  },
  {
    name: 'dynamic route',
    method: 'GET',
    path: '/user/lookup/username/hey',
  },
  {
    name: 'mixed static dynamic',
    method: 'GET',
    path: '/event/abcd1234/comments',
  },
  {
    name: 'post',

Review comment (bug):
This is wrong. `buildInitParams` accepts `paths: string[]`. So the correct code is:

```ts
paths: routes.map((r) => r.path)
```

Code after:
import MedleyRouter from '@medley/router'
import type { HTTPMethod } from 'find-my-way'
import findMyWay from 'find-my-way'
import KoaRouter from 'koa-tree-router'
import { run, bench, group } from 'mitata'
import TrekRouter from 'trek-router'
import { LinearRouter } from '../../../src/router/linear-router/index.ts'
import {
  RegExpRouter,
  PreparedRegExpRouter,
  buildInitParams,
} from '../../../src/router/reg-exp-router/index.ts'
import { TrieRouter } from '../../../src/router/trie-router/index.ts'
import type { Route } from './tool.mts'
import { routes } from './tool.mts'

const preparedParams = buildInitParams({
  paths: routes.map((r) => r.path),
})

const benchRoutes: (Route & { name: string })[] = [
  {
    name: 'short static',
    method: 'GET',
    path: '/user',
  },
  {
    name: 'static with same radix',
    method: 'GET',
    path: '/user/comments',
  },
  {
    name: 'dynamic route',
    method: 'GET',
    path: '/user/lookup/username/hey',
  },
  {
    name: 'mixed static dynamic',
    method: 'GET',
    path: '/event/abcd1234/comments',
  },
  {
    name: 'post',
Repository: honojs/hono
File: src/middleware/csrf/index.ts (TypeScript)

Code before:
    } else if (typeof optsOrigin === 'string') {
      return (origin) => origin === optsOrigin
    } else if (typeof optsOrigin === 'function') {
      return optsOrigin
    } else {
      return (origin) => optsOrigin.includes(origin)
    }
  })(options?.origin)
  const isAllowedOrigin = (origin: string | undefined, c: Context) => {
    if (origin === undefined) {
      // denied always when origin header is not present
      return false
    }
    return handler(origin, c)
  }

  return async function cors(c, next) {
    if (
      !isSafeMethodRe.test(c.req.method) &&
      isRequestedByFormElementRe.test(c.req.header('content-type') || '') &&
      !isAllowedOrigin(c.req.header('origin'), c)
    ) {
      const res = new Response('Forbidden', {
        status: 403,
      })
      throw new HTTPException(401, { res })
    }

    await next()
  }
}


Review comment (bug):
I think that `401` should be `403`.

P.S.

However, if it remains as `401`, the tests do not fail. This is because if `{ res }` is set, it can be used and the status number in the first argument of `HTTPException` is ignored. Therefore, there is no problem for real use cases with this code. But, for making it the same as other middleware, I think it should be changed to `403`.

Code after:
    } else if (typeof optsOrigin === 'string') {
      return (origin) => origin === optsOrigin
    } else if (typeof optsOrigin === 'function') {
      return optsOrigin
    } else {
      return (origin) => optsOrigin.includes(origin)
    }
  })(options?.origin)
  const isAllowedOrigin = (origin: string | undefined, c: Context) => {
    if (origin === undefined) {
      // denied always when origin header is not present
      return false
    }
    return handler(origin, c)
  }

  return async function cors(c, next) {
    if (
      !isSafeMethodRe.test(c.req.method) &&
      isRequestedByFormElementRe.test(c.req.header('content-type') || '') &&
      !isAllowedOrigin(c.req.header('origin'), c)
    ) {
      const res = new Response('Forbidden', {
        status: 403,
      })
      throw new HTTPException(403, { res })
    }

    await next()
  }
}