ronantakizawa/codereview-bench huggingface dataset:
Repository: honojs/hono
File: src/request.ts (TypeScript)

Code before:
    if (anyCachedKey) {
      return (bodyCache[anyCachedKey as keyof Body] as Promise<BodyInit>).then((body) => {
        if (anyCachedKey === 'json') {
          body = JSON.stringify(body)
        }
        return new Response(body)[key]()
      })
    }

    return (bodyCache[key] = raw[key]())
  }

  /**
   * `.json()` can parse Request body of type `application/json`
   *
   * @see {@link https://hono.dev/docs/api/request#json}
   *
   * @example
   * ```ts
   * app.post('/entry', async (c) => {
   *   const body = await c.req.json()
   * })
   * ```
   */
  json<T = any>(): Promise<T> {
    return this.#cachedBody('json')
  }

  /**
   * `.text()` can parse Request body of type `text/plain`
   *
   * @see {@link https://hono.dev/docs/api/request#text}
   *
   * @example
   * ```ts
   * app.post('/entry', async (c) => {
   *   const body = await c.req.text()
   * })
   * ```
   */
  text(): Promise<string> {
    return this.#cachedBody('text')
  }

  /**
   * `.arrayBuffer()` parse Request body as an `ArrayBuffer`
   *
   * @see {@link https://hono.dev/docs/api/request#arraybuffer}
   *
   * @example
   * ```ts

Review comment (bug):
@yusukebe 

I think this broken `req.bodyCache.json` ?

I got `undefined` for `req.bodyCache.json` after this commit

Code after:
    if (anyCachedKey) {
      return (bodyCache[anyCachedKey as keyof Body] as Promise<BodyInit>).then((body) => {
        if (anyCachedKey === 'json') {
          body = JSON.stringify(body)
        }
        return new Response(body)[key]()
      })
    }

    return (bodyCache[key] = raw[key]())
  }

  /**
   * `.json()` can parse Request body of type `application/json`
   *
   * @see {@link https://hono.dev/docs/api/request#json}
   *
   * @example
   * ```ts
   * app.post('/entry', async (c) => {
   *   const body = await c.req.json()
   * })
   * ```
   */
  json<T = any>(): Promise<T> {
    return this.#cachedBody('text').then((text: string) => JSON.parse(text))
  }

  /**
   * `.text()` can parse Request body of type `text/plain`
   *
   * @see {@link https://hono.dev/docs/api/request#text}
   *
   * @example
   * ```ts
   * app.post('/entry', async (c) => {
   *   const body = await c.req.text()
   * })
   * ```
   */
  text(): Promise<string> {
    return this.#cachedBody('text')
  }

  /**
   * `.arrayBuffer()` parse Request body as an `ArrayBuffer`
   *
   * @see {@link https://hono.dev/docs/api/request#arraybuffer}
   *
   * @example
   * ```ts
Repository: honojs/hono
File: src/middleware/serve-static/path.ts (TypeScript)

Code before:
const isWindowsPath = (path: string): boolean => {
  return /^[a-zA-Z]:/.test(path) || path.includes('\\')
}

export const defaultJoin = (...paths: string[]): string => {
  if (paths.length === 0) return '.'

  const isWindows = paths.some(isWindowsPath)
  const sep = isWindows ? '\\' : '/'
  const otherSep = isWindows ? '/' : '\\'

  let result = paths[0] || ''

  for (let i = 1; i < paths.length; i++) {
    const segment = paths[i]
    if (!segment) continue

    if (result && !result.endsWith(sep) && !result.endsWith(otherSep)) {
      result += sep
    }
    result += segment
  }

  result = result.replace(/[/\\]+/g, sep)

  if (result.startsWith(`.${sep}`)) {
    result = result.slice(2)
  }

  const segments = result.split(/[/\\]/)
  const resolvedSegments: string[] = []

Review comment (suggestion):
```suggestion
  if (paths.length === 0) {
    return '.'
  }
```

Until now, at hono, we have always used `{` with `if`, so I think it would be better to use it here as well.

Code after:
/**
 * `defaultJoin` does not support Windows paths and always uses `/` separators.
 * If you need Windows path support, please use `join` exported from `node:path` etc. instead.
 */
export const defaultJoin = (...paths: string[]): string => {
  // Join non-empty paths with '/'
  let result = paths.filter((p) => p !== '').join('/')

  // Normalize multiple slashes to single slash
  result = result.replace(/(?<=\/)\/+/g, '')

  // Handle path resolution (. and ..)
  const segments = result.split('/')
  const resolved = []

  for (const segment of segments) {
    if (segment === '..' && resolved.length > 0 && resolved.at(-1) !== '..') {
      resolved.pop()
    } else if (segment !== '.') {
      resolved.push(segment)
    }
  }

  return resolved.join('/') || '.'
}
Repository: honojs/hono
File: src/utils/url.ts (TypeScript)

Code before:
  url: string,
  key?: string
) => string | undefined | Record<string, string>

export const getQueryParams = (
  url: string,
  key?: string
): string[] | undefined | Record<string, string[]> => {
  return _getQueryParam(url, key, true) as string[] | undefined | Record<string, string[]>
}

// `decodeURIComponent` is a long name.
// By making it a function, we can use it commonly when minified, reducing the amount of code.
export const decodeURIComponent_ = decodeURIComponent

/**
 * This function ensures that the string is safely encoded,
 * and not double-encoded.
 * The input string may or may not escaped by encodeURI.
 */
export const safeEncodeURI = (str: string): string => {
  try {
    // decodeURI will throw if str contains %
    // In that case, we can assume that str is not encoded
    return encodeURI(decodeURI(str))
  } catch (e) {
    if (e instanceof URIError) {
      return encodeURI(str)
    }
    throw e
  }
}


Review comment (suggestion):
It looks like it should be `!==` instead of `===` here @yusukebe 
```suggestion
    // If successful, encode only if it's different from the original
    return decoded !== str ? encodeURI(str) : str
```

Code after:
  url: string,
  key?: string
) => string | undefined | Record<string, string>

export const getQueryParams = (
  url: string,
  key?: string
): string[] | undefined | Record<string, string[]> => {
  return _getQueryParam(url, key, true) as string[] | undefined | Record<string, string[]>
}

// `decodeURIComponent` is a long name.
// By making it a function, we can use it commonly when minified, reducing the amount of code.
export const decodeURIComponent_ = decodeURIComponent

/**
 * This function ensures that the string is safely encoded,
 * and not double-encoded.
 * The input string may or may not escaped by encodeURI.
 */
export const safeEncodeURI = (str: string): string => {
  try {
    // Try to decode the string
    const decoded = decodeURI(str)
    // If successful, encode only if it's different from the original
    return decoded === str ? encodeURI(str) : str
  } catch (e) {
    if (e instanceof URIError) {
      // If decoding fails, the string is not encoded, so encode it
      return encodeURI(str)
    }
    throw e
  }
}
Repository: honojs/hono
File: src/client/utils.test.ts (TypeScript)

Code before:
import { HttpResponse, http } from 'msw'
import { setupServer } from 'msw/node'
import { Hono } from '../hono'
import type { Expect, Equal } from '../utils/types'
import { hc } from './client'
import {
  buildSearchParams,
  deepMerge,
  hcParse,
  mergePath,
  removeIndexString,
  replaceUrlParam,
  replaceUrlProtocol,
} from './utils'

describe('mergePath', () => {
  it('Should merge paths correctly', () => {
    expect(mergePath('http://localhost', '/api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost/', '/api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost', 'api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost/', 'api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost/', '/')).toBe('http://localhost/')
  })
})

describe('replaceUrlParams', () => {

Review comment (suggestion):
I updated the test to use `msw` instead of running the actual server. This is the same way as `src/client/client.test.ts`.

Code after:
import { HttpResponse, http } from 'msw'
import { setupServer } from 'msw/node'
import { Hono } from '../hono'
import type { Expect, Equal } from '../utils/types'
import { hc } from './client'
import {
  buildSearchParams,
  deepMerge,
  parseResponse,
  mergePath,
  removeIndexString,
  replaceUrlParam,
  replaceUrlProtocol,
} from './utils'

describe('mergePath', () => {
  it('Should merge paths correctly', () => {
    expect(mergePath('http://localhost', '/api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost/', '/api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost', 'api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost/', 'api')).toBe('http://localhost/api')
    expect(mergePath('http://localhost/', '/')).toBe('http://localhost/')
  })
})

describe('replaceUrlParams', () => {
Repository: honojs/hono
File: src/client/utils.ts (TypeScript)

Code before:
  }
  const merged = { ...target } as ObjectType<T>

  for (const key in source) {
    const value = source[key]
    if (isObject(merged[key]) && isObject(value)) {
      merged[key] = deepMerge(merged[key], value)
    } else {
      merged[key] = value as T[keyof T] & T
    }
  }

  return merged as T
}

/**
 * Shortcut to get a consumable response from `hc` fetch calls, with types inference.
 *
 * Smartly parse the response data, and automatically throw an error if the response is not ok.
 *
 * To handle an error, see {@link DetailedError} interface.
 *
 * @example hcParse(client.posts.$get())
 */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export async function hcParse<T extends ClientResponse<any>>(
  fetchRes: T | Promise<T>
): Promise<
  T extends ClientResponse<infer RT, infer _, infer RF>
    ? RF extends 'json'
      ? RT
      : RT extends string
      ? RT
      : string
    : never
> {
  return fetchRP(fetchRes)
}


Review comment (suggestion):
I like the name `parseResponse` simply. `parseResponse` uses the general word `response`, but the user can understand it as `hc` related because it's exported from `hono/client`.

```ts
import { hc, parseResponse } from 'hono/client'
```

Code after:
  }
  const merged = { ...target } as ObjectType<T>

  for (const key in source) {
    const value = source[key]
    if (isObject(merged[key]) && isObject(value)) {
      merged[key] = deepMerge(merged[key], value)
    } else {
      merged[key] = value as T[keyof T] & T
    }
  }

  return merged as T
}

/**
 * Shortcut to get a consumable response from `hc`'s fetch calls (Response), with types inference.
 *
 * Smartly parse the response data, throwing a structured error if the response is not `ok`. ({@link DetailedError})
 *
 * @example const result = await parseResponse(client.posts.$get())
 */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export async function parseResponse<T extends ClientResponse<any>>(
  fetchRes: T | Promise<T>
): Promise<
  T extends ClientResponse<infer RT, infer _, infer RF>
    ? RF extends 'json'
      ? RT
      : RT extends string
      ? RT
      : string
    : never
> {
  return fetchRP(fetchRes)
}
Repository: honojs/hono
File: vitest.config.ts (TypeScript)

Code before:
          jsx: 'automatic',
          jsxImportSource: './src/jsx',
        },
        extends: true,
        test: {
          environment: 'node',
          exclude: [...configDefaults.exclude, 'src/jsx', '**/*.case.test.*'],
          include: [
            'src/**/(*.)+(spec|test).+(ts|tsx|js)',
            'scripts/**/(*.)+(spec|test).+(ts|tsx|js)',
            'build/**/(*.)+(spec|test).+(ts|tsx|js)',
          ],
          name: 'unit',
        },
      },
      {
        esbuild: {
          jsx: 'automatic',
          jsxImportSource: './src/jsx',
        },
        extends: true,
        test: {
          include: ['src/jsx/dom/**/(*.)+(spec|test).+(ts|tsx|js)', 'src/jsx/hooks/dom.test.tsx'],
          name: 'jsx-runtime-default',
        },
      },
      {
        esbuild: {
          jsx: 'automatic',
          jsxImportSource: './src/jsx/dom',
        },
        extends: true,
        test: {
          include: ['src/jsx/dom/**/(*.)+(spec|test).+(ts|tsx|js)', 'src/jsx/hooks/dom.test.tsx'],
          name: 'jsx-runtime-dom',
        },
      },
    ],
  },
})


Review comment (suggestion):
This runs the same tests as what `vitest -c .vitest.config/jsx-runtime-default.ts --run` previously ran

Code after:
        esbuild: {
          jsx: 'automatic',
          jsxImportSource: './src/jsx',
        },
        extends: true,
        test: {
          exclude: [...configDefaults.exclude, '**/sandbox/**', '**/*.case.test.*'],
          include: [
            'src/**/(*.)+(spec|test).+(ts|tsx|js)',
            'scripts/**/(*.)+(spec|test).+(ts|tsx|js)',
            'build/**/(*.)+(spec|test).+(ts|tsx|js)',
          ],
          name: 'main',
        },
      },
      {
        esbuild: {
          jsx: 'automatic',
          jsxImportSource: './src/jsx',
        },
        extends: true,
        test: {
          include: ['src/jsx/dom/**/(*.)+(spec|test).+(ts|tsx|js)', 'src/jsx/hooks/dom.test.tsx'],
          name: 'jsx-runtime-default',
        },
      },
      {
        esbuild: {
          jsx: 'automatic',
          jsxImportSource: './src/jsx/dom',
        },
        extends: true,
        test: {
          include: ['src/jsx/dom/**/(*.)+(spec|test).+(ts|tsx|js)', 'src/jsx/hooks/dom.test.tsx'],
          name: 'jsx-runtime-dom',
        },
      },
    ],
  },
})
Repository: honojs/hono
File: src/helper/proxy/index.ts (TypeScript)

Code before:
  'keep-alive',
  'proxy-authenticate',
  'proxy-authorization',
  'te',
  'trailers',
  'transfer-encoding',
]

interface ProxyRequestInit extends Omit<RequestInit, 'headers'> {
  raw?: Request
  headers?:
    | HeadersInit
    | [string, string][]
    | Record<RequestHeader, string | undefined>
    | Record<string, string | undefined>
}

/**
 * Options for proxy function
 * @example
 * { fetch: customFetch } // Use custom fetch function instead of the global `fetch`
 */
type ProxyOptions = {
  fetch: typeof fetch
}

interface ProxyFetch {
  (
    input: string | URL | Request,
    init?: ProxyRequestInit,
    options?: ProxyOptions
  ): Promise<Response>
}

const buildRequestInitFromRequest = (
  request: Request | undefined
): RequestInit & { duplex?: 'half' } => {
  if (!request) {
    return {}
  }

  const headers = new Headers(request.headers)
  hopByHopHeaders.forEach((header) => {
    headers.delete(header)
  })

  return {
    method: request.method,
    body: request.body,
    duplex: request.body ? 'half' : undefined,
    headers,

Review comment (security):
I like what `oauth4webapi` does with a [`customFetch`](https://github.com/panva/oauth4webapi/blob/main/docs/variables/customFetch.md) option as a `unique symbol`

```ts
import { customFetch } from 'hono'
import { proxy } from 'hono/proxy'

proxy(`https://example.com/`, {
  [customFetch](...args) {
    return c.env.AUTH.fetch(...args)
  }
})
```

At least that way you could avoid adding a third argument?

Code after:
  'keep-alive',
  'proxy-authenticate',
  'proxy-authorization',
  'te',
  'trailers',
  'transfer-encoding',
]

interface ProxyRequestInit extends Omit<RequestInit, 'headers'> {
  raw?: Request
  headers?:
    | HeadersInit
    | [string, string][]
    | Record<RequestHeader, string | undefined>
    | Record<string, string | undefined>
  customFetch?: (request: Request) => Promise<Response>
}

interface ProxyFetch {
  (input: string | URL | Request, init?: ProxyRequestInit): Promise<Response>
}

const buildRequestInitFromRequest = (
  request: Request | undefined
): RequestInit & { duplex?: 'half' } => {
  if (!request) {
    return {}
  }

  const headers = new Headers(request.headers)
  hopByHopHeaders.forEach((header) => {
    headers.delete(header)
  })

  return {
    method: request.method,
    body: request.body,
    duplex: request.body ? 'half' : undefined,
    headers,
    signal: request.signal,
  }
}

const preprocessRequestInit = (requestInit: RequestInit): RequestInit => {
  if (
    !requestInit.headers ||
    Array.isArray(requestInit.headers) ||
    requestInit.headers instanceof Headers
  ) {
    return requestInit
  }
Repository: honojs/hono
File: src/helper/websocket/index.ts (TypeScript)

Code before:
 * A context for controlling WebSockets
 */
export class WSContext<T = unknown> {
  #init: WSContextInit<T>
  constructor(init: WSContextInit<T>) {
    this.#init = init
    this.raw = init.raw
    this.url = init.url ? new URL(init.url) : null
    this.protocol = init.protocol ?? null
  }
  send(source: string | ArrayBuffer | Uint8Array<ArrayBuffer>, options?: SendOptions): void {
    this.#init.send(source, options ?? {})
  }
  raw?: T
  binaryType: BinaryType = 'arraybuffer'
  get readyState(): WSReadyState {
    return this.#init.readyState
  }
  url: URL | null
  protocol: string | null
  close(code?: number, reason?: string) {
    this.#init.close(code, reason)
  }
}

export type WSMessageReceive =
  | string
  | Blob
  | ArrayBuffer
  | SharedArrayBuffer
  | Uint8Array<ArrayBuffer>

export const createWSMessageEvent = (source: WSMessageReceive): MessageEvent<WSMessageReceive> => {
  return new MessageEvent<WSMessageReceive>('message', {
    data: source,
  })
}

export interface WebSocketHelperDefineContext {}
export type WebSocketHelperDefineHandler<T, U> = (
  c: Context,
  events: WSEvents<T>,
  options?: U
) => Promise<Response | void> | Response | void

/**
 * Create a WebSocket adapter/helper
 */
export const defineWebSocketHelper = <T = unknown, U = any>(
  handler: WebSocketHelperDefineHandler<T, U>
): UpgradeWebSocket<T, U> => {

Review comment (suggestion):
Replaced `ArrayBufferLike` with explicit types (`ArrayBuffer | SharedArrayBuffer | Uint8Array<ArrayBuffer>`) to support all buffer types.

Code after:
 * A context for controlling WebSockets
 */
export class WSContext<T = unknown> {
  #init: WSContextInit<T>
  constructor(init: WSContextInit<T>) {
    this.#init = init
    this.raw = init.raw
    this.url = init.url ? new URL(init.url) : null
    this.protocol = init.protocol ?? null
  }
  send(source: string | ArrayBuffer | Uint8Array<ArrayBuffer>, options?: SendOptions): void {
    this.#init.send(source, options ?? {})
  }
  raw?: T
  binaryType: BinaryType = 'arraybuffer'
  get readyState(): WSReadyState {
    return this.#init.readyState
  }
  url: URL | null
  protocol: string | null
  close(code?: number, reason?: string) {
    this.#init.close(code, reason)
  }
}

export type WSMessageReceive = string | Blob | ArrayBufferLike

export const createWSMessageEvent = (source: WSMessageReceive): MessageEvent<WSMessageReceive> => {
  return new MessageEvent<WSMessageReceive>('message', {
    data: source,
  })
}

export interface WebSocketHelperDefineContext {}
export type WebSocketHelperDefineHandler<T, U> = (
  c: Context,
  events: WSEvents<T>,
  options?: U
) => Promise<Response | void> | Response | void

/**
 * Create a WebSocket adapter/helper
 */
export const defineWebSocketHelper = <T = unknown, U = any>(
  handler: WebSocketHelperDefineHandler<T, U>
): UpgradeWebSocket<T, U> => {
  return ((
    ...args:
      | [createEvents: (c: Context) => WSEvents<T> | Promise<WSEvents<T>>, options?: U]
      | [c: Context, events: WSEvents<T>, options?: U]
  ) => {
Repository: honojs/hono
File: src/utils/jwt/jwt.test.ts (TypeScript)

Code before:

    let err = null
    let authorized
    try {
      authorized = await JWT.verify(tok, secret + 'invalid', AlgorithmTypes.HS256)
    } catch (e) {
      err = e
    }
    expect(authorized).toBeUndefined()
    expect(err instanceof JwtTokenSignatureMismatched).toBe(true)
  })

  it('sign & verify & decode with a custom secret', async () => {
    const payload = { message: 'hello world' }
    const algorithm = {
      name: 'HMAC',
      hash: {
        name: 'SHA-256',
      },
    }
    const secret = await crypto.subtle.importKey(
      'raw',
      Buffer.from(
        'cefb73234d5fae4bf27662900732b52943e8d53e871fe0f353da95de4599c21d',
        'hex'
      ) as BufferSource,
      algorithm,
      false,
      ['sign', 'verify']
    )
    const tok = await JWT.sign(payload, secret)
    const expected =
      'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJtZXNzYWdlIjoiaGVsbG8gd29ybGQifQ.qunGhchNXH_unqWXN6hB0Elhzr5SykSXVhklLti1aFI'
    expect(tok).toEqual(expected)

    const verifiedPayload = await JWT.verify(tok, secret)
    expect(verifiedPayload).not.toBeUndefined()
    expect(verifiedPayload).toEqual(payload)

    const invalidSecret = await crypto.subtle.importKey(
      'raw',
      Buffer.from(
        'cefb73234d5fae4bf27662900732b52943e8d53e871fe0f353da95de41111111',
        'hex'
      ) as BufferSource,
      algorithm,
      false,
      ['sign', 'verify']
    )
    let err = null
    let authorized

Review comment (suggestion):
`Buffer.from()` returns a Node.js Buffer whose type doesn't directly match `BufferSource` in TypeScript 5.9.

Code after:

    let err = null
    let authorized
    try {
      authorized = await JWT.verify(tok, secret + 'invalid', AlgorithmTypes.HS256)
    } catch (e) {
      err = e
    }
    expect(authorized).toBeUndefined()
    expect(err instanceof JwtTokenSignatureMismatched).toBe(true)
  })

  it('sign & verify & decode with a custom secret', async () => {
    const payload = { message: 'hello world' }
    const algorithm = {
      name: 'HMAC',
      hash: {
        name: 'SHA-256',
      },
    }
    const secret = await crypto.subtle.importKey(
      'raw',
      Buffer.from('cefb73234d5fae4bf27662900732b52943e8d53e871fe0f353da95de4599c21d', 'hex'),
      algorithm,
      false,
      ['sign', 'verify']
    )
    const tok = await JWT.sign(payload, secret)
    const expected =
      'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJtZXNzYWdlIjoiaGVsbG8gd29ybGQifQ.qunGhchNXH_unqWXN6hB0Elhzr5SykSXVhklLti1aFI'
    expect(tok).toEqual(expected)

    const verifiedPayload = await JWT.verify(tok, secret)
    expect(verifiedPayload).not.toBeUndefined()
    expect(verifiedPayload).toEqual(payload)

    const invalidSecret = await crypto.subtle.importKey(
      'raw',
      Buffer.from('cefb73234d5fae4bf27662900732b52943e8d53e871fe0f353da95de41111111', 'hex'),
      algorithm,
      false,
      ['sign', 'verify']
    )
    let err = null
    let authorized
    try {
      authorized = await JWT.verify(tok, invalidSecret)
    } catch (e) {
      err = e
    }
    expect(authorized).toBeUndefined()