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

Code before:
/**
 * @module
 * Proxy Helper for Hono.
 */

/*
 * The following headers will be deleted from the response:
 * * Content-Encoding
 * * Content-Length
 * * Content-Range
 */
const forceDeleteResponseHeaderNames = ['Content-Encoding', 'Content-Length', 'Content-Range']

// Typical header names for requests for proxy use
type ProxyRequestHeaderName = 'X-Forwarded-For' | 'X-Forwarded-Proto' | 'X-Forwarded-Host'

interface ProxyRequestInit extends RequestInit {
  /**
   * Headers that are overwritten in requests to the origin server.
   * Specify undefined to delete the header.
   */
  proxySetRequestHeaders?: Partial<Record<ProxyRequestHeaderName, string>> &
    Record<string, string | undefined>
  /**
   * Headers included in the response from the origin server that should be removed in the response to the client.
   */
  proxyDeleteResponseHeaderNames?: string[]
}

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

/**
 * Fetch API wrapper for proxy.
 * The parameters and return value are the same as for `fetch` (except for the proxy-specific options).

Review comment (bug):
i'm not sure about `Content-Length` and `Content-Range`, as if the original request is for a byte range, the semantics is not preserved. a probable semantically correct implementation:

1. if the original request contains `Range` header, fast-fail with 400 status code
1. strip `Accept-Range` from response header
2. indicate upstream error if response header contain `Content-Range`

`Content-Length` should be left alone in all case IMO

Code after:
/**
 * @module
 * Proxy Helper for Hono.
 */

import type { RequestHeader } from '../../utils/headers'

// https://datatracker.ietf.org/doc/html/rfc2616#section-13.5.1
const hopByHopHeaders = [
  'connection',
  '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>
}

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

const buildRequestInitFromRequest = (
  request: Request | undefined
): RequestInit & { duplex?: 'half' } => {
  if (!request) {
    return {}
  }
Repository: honojs/hono
File: src/client/client.ts (TypeScript)

Code before:
        for (const [k, v] of Object.entries(args.form)) {
          if (Array.isArray(v)) {
            for (const v2 of v) {
              form.append(k, v2)
            }
          } else {
            form.append(k, v)
          }
        }
        this.rBody = form
      }

      if (args.json) {
        this.rBody = JSON.stringify(args.json)
        this.cType = 'application/json'
      }

      if (args.param) {
        this.pathParams = args.param
      }
    }

    let methodUpperCase = this.method.toUpperCase()

    const headerValues: Record<string, string> = {
      ...args?.header,
      ...(typeof opt?.headers === 'function'
        ? await opt.headers()
        : opt?.headers
        ? opt.headers
        : {}),
    }

    if (args?.cookie) {
      const cookies: string[] = []
      for (const [key, value] of Object.entries(args.cookie)) {
        cookies.push(serialize(key, value, { path: '/' }))
      }
      headerValues['Cookie'] = cookies.join(',')
    }

    if (this.cType) {
      headerValues['Content-Type'] = this.cType
    }

    const headers = new Headers(headerValues ?? undefined)
    let url = this.url

    url = removeIndexString(url)
    url = replaceUrlParam(url, this.pathParams)


Review comment (suggestion):
Note) spread fallback is redundant

```typescript
const a = undefined
console.log({ ...a, b: 1 })

// { b: 1}
```

Code after:
        for (const [k, v] of Object.entries(args.form)) {
          if (Array.isArray(v)) {
            for (const v2 of v) {
              form.append(k, v2)
            }
          } else {
            form.append(k, v)
          }
        }
        this.rBody = form
      }

      if (args.json) {
        this.rBody = JSON.stringify(args.json)
        this.cType = 'application/json'
      }

      if (args.param) {
        this.pathParams = args.param
      }
    }

    let methodUpperCase = this.method.toUpperCase()

    const headerValues: Record<string, string> = {
      ...args?.header,
      ...(typeof opt?.headers === 'function' ? await opt.headers() : opt?.headers),
    }

    if (args?.cookie) {
      const cookies: string[] = []
      for (const [key, value] of Object.entries(args.cookie)) {
        cookies.push(serialize(key, value, { path: '/' }))
      }
      headerValues['Cookie'] = cookies.join(',')
    }

    if (this.cType) {
      headerValues['Content-Type'] = this.cType
    }

    const headers = new Headers(headerValues ?? undefined)
    let url = this.url

    url = removeIndexString(url)
    url = replaceUrlParam(url, this.pathParams)

    if (this.queryParams) {
      url = url + '?' + this.queryParams.toString()
    }
    methodUpperCase = this.method.toUpperCase()
Repository: honojs/hono
File: src/context.ts (TypeScript)

Code before:
    return typeof arg === 'number'
      ? this.newResponse(html as string, arg, headers)
      : this.newResponse(html as string, arg)
  }

  /**
   * `.redirect()` can Redirect, default status code is 302.
   *
   * @see {@link https://hono.dev/docs/api/context#redirect}
   *
   * @example
   * ```ts
   * app.get('/redirect', (c) => {
   *   return c.redirect('/')
   * })
   * app.get('/redirect-permanently', (c) => {
   *   return c.redirect('/', 301)
   * })
   * ```
   */
  redirect = <T extends RedirectStatusCode = 302>(
    location: string | URL,
    status?: T
  ): Response & TypedResponse<undefined, T, 'redirect'> => {
    this.#headers ??= new Headers()
    this.#headers.set('Location', location.toString())
    return this.newResponse(null, status ?? 302) as any
  }

  /**
   * `.notFound()` can return the Not Found Response.
   *
   * @see {@link https://hono.dev/docs/api/context#notfound}
   *
   * @example
   * ```ts
   * app.get('/notfound', (c) => {
   *   return c.notFound()
   * })
   * ```
   */
  notFound = (): Response | Promise<Response> => {
    this.#notFoundHandler ??= () => new Response()
    return this.#notFoundHandler(this)
  }
}


Review comment (suggestion):
```suggestion
    this.#headers.set('Location', String(location))
```

Although slight, this change reduces the amount of code while maintaining the same operation.

Code after:
    return typeof arg === 'number'
      ? this.newResponse(html as string, arg, headers)
      : this.newResponse(html as string, arg)
  }

  /**
   * `.redirect()` can Redirect, default status code is 302.
   *
   * @see {@link https://hono.dev/docs/api/context#redirect}
   *
   * @example
   * ```ts
   * app.get('/redirect', (c) => {
   *   return c.redirect('/')
   * })
   * app.get('/redirect-permanently', (c) => {
   *   return c.redirect('/', 301)
   * })
   * ```
   */
  redirect = <T extends RedirectStatusCode = 302>(
    location: string | URL,
    status?: T
  ): Response & TypedResponse<undefined, T, 'redirect'> => {
    this.#headers ??= new Headers()
    this.#headers.set('Location', String(location))
    return this.newResponse(null, status ?? 302) as any
  }

  /**
   * `.notFound()` can return the Not Found Response.
   *
   * @see {@link https://hono.dev/docs/api/context#notfound}
   *
   * @example
   * ```ts
   * app.get('/notfound', (c) => {
   *   return c.notFound()
   * })
   * ```
   */
  notFound = (): Response | Promise<Response> => {
    this.#notFoundHandler ??= () => new Response()
    return this.#notFoundHandler(this)
  }
}
Repository: honojs/hono
File: perf-measures/bundle-check/scripts/check-bundle-size.ts (TypeScript)

Code before:
  const tempFilePath = path.join(tempDir, 'bundle.tmp.js')

  try {
    await esbuild.build({
      entryPoints: ['dist/index.js'],
      bundle: true,
      minify: true,
      format: 'esm' as esbuild.Format,
      target: 'es2022',
      outfile: tempFilePath,
    })

    const bundleSize = fs.statSync(tempFilePath).size
    const metrics = []

    metrics.push({
      key: 'bundle-size-b',
      name: 'Bundle Size (B)',
      value: bundleSize,
      unit: 'B',
    })

    metrics.push({
      key: 'bundle-size-kb',
      name: 'Bundle Size (KB)',
      value: (bundleSize / 1024).toFixed(2).toString(),
      unit: 'KB',
    })

    const benchmark = {
      key: 'bundle-size-check',
      name: 'Bundle size check',
      metrics,
    }
    console.log(JSON.stringify(benchmark, null, 2))
  } catch (error) {
    console.error('Build failed:', error)
  } finally {
    if (fs.existsSync(tempFilePath)) {
      fs.unlinkSync(tempFilePath)
    }
  }
}

main()


Review comment (suggestion):
```suggestion
      value: parseFloat((bundleSize / 1024).toFixed(2)),
```
It looks like it had to be Number in the custom metrics schema.
https://github.com/k1LoW/octocov/blob/main/report/custom_metrics_schema.json#L51-L53

Code after:
  const tempFilePath = path.join(tempDir, 'bundle.tmp.js')

  try {
    await esbuild.build({
      entryPoints: ['dist/index.js'],
      bundle: true,
      minify: true,
      format: 'esm' as esbuild.Format,
      target: 'es2022',
      outfile: tempFilePath,
    })

    const bundleSize = fs.statSync(tempFilePath).size
    const metrics = []

    metrics.push({
      key: 'bundle-size-b',
      name: 'Bundle Size (B)',
      value: bundleSize,
      unit: 'B',
    })

    metrics.push({
      key: 'bundle-size-kb',
      name: 'Bundle Size (KB)',
      value: parseFloat((bundleSize / 1024).toFixed(2)),
      unit: 'K',
    })

    const benchmark = {
      key: 'bundle-size-check',
      name: 'Bundle size check',
      metrics,
    }
    console.log(JSON.stringify(benchmark, null, 2))
  } catch (error) {
    console.error('Build failed:', error)
  } finally {
    if (fs.existsSync(tempFilePath)) {
      fs.unlinkSync(tempFilePath)
    }
  }
}

main()
Repository: honojs/hono
File: src/middleware/logger/index.ts (TypeScript)

Code before:
      : `${prefix} ${method} ${path} ${colorStatus(status)} ${elapsed}`
  fn(out)
}

/**
 * Logger Middleware for Hono.
 *
 * @see {@link https://hono.dev/docs/middleware/builtin/logger}
 *
 * @param {PrintFunc} [fn=console.log] - Optional function for customized logging behavior.
 * @returns {MiddlewareHandler} The middleware handler function.
 *
 * @example
 * ```ts
 * const app = new Hono()
 *
 * app.use(logger())
 * app.get('/', (c) => c.text('Hello Hono!'))
 * ```
 */
export const logger = (fn: PrintFunc = console.log): MiddlewareHandler => {
  return async function logger(c, next) {
    const { method } = c.req

    const url = new URL(c.req.raw.url)
    const path = url.pathname + url.search

    log(fn, LogPrefix.Incoming, method, path)

    const start = Date.now()

    await next()

    log(fn, LogPrefix.Outgoing, method, path, c.res.status, time(start))
  }
}


Review comment (suggestion):
If an attacker intentionally sends a large size of query parameters,
I think this should be optional as it will contaminate the standard output and server overload due to query analysis time.

Code after:
      : `${prefix} ${method} ${path} ${colorStatus(status)} ${elapsed}`
  fn(out)
}

/**
 * Logger Middleware for Hono.
 *
 * @see {@link https://hono.dev/docs/middleware/builtin/logger}
 *
 * @param {PrintFunc} [fn=console.log] - Optional function for customized logging behavior.
 * @returns {MiddlewareHandler} The middleware handler function.
 *
 * @example
 * ```ts
 * const app = new Hono()
 *
 * app.use(logger())
 * app.get('/', (c) => c.text('Hello Hono!'))
 * ```
 */
export const logger = (fn: PrintFunc = console.log): MiddlewareHandler => {
  return async function logger(c, next) {
    const { method, url } = c.req

    const path = url.slice(url.indexOf('/', 8))

    log(fn, LogPrefix.Incoming, method, path)

    const start = Date.now()

    await next()

    log(fn, LogPrefix.Outgoing, method, path, c.res.status, time(start))
  }
}
Repository: honojs/hono
File: src/context.ts (TypeScript)

Code before:
interface SetHeadersOptions {
  append?: boolean
}

interface SetHeaders {
  (name: 'Content-Type', value?: BaseMime, options?: SetHeadersOptions): void
  (name: ResponseHeader, value?: string, options?: SetHeadersOptions): void
  (name: string, value?: string, options?: SetHeadersOptions): void
}

type ResponseHeadersInit =
  | [string, string][]
  | Record<'Content-Type', BaseMime>
  | Record<ResponseHeader, string>
  | Record<string, string>
  | Headers

interface ResponseInit {
  headers?: ResponseHeadersInit
  status?: number
  statusText?: string
}

export const TEXT_PLAIN = 'text/plain; charset=UTF-8'

/**
 * Sets the headers of a response.
 *
 * @param headers - The Headers object to set the headers on.
 * @param map - A record of header key-value pairs to set.
 * @returns The updated Headers object.
 */
const setHeaders = (headers: Headers, map: Record<string, string> = {}) => {
  for (const key of Object.keys(map)) {
    headers.set(key, map[key])
  }
  return headers
}

export class Context<
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  E extends Env = any,
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  P extends string = any,
  I extends Input = {}
> {
  #rawRequest: Request
  #req: HonoRequest<P, I['out']> | undefined
  /**
   * `.env` can get bindings (environment variables, secrets, KV namespaces, D1 database, R2 bucket etc.) in Cloudflare Workers.
   *

Review comment (suggestion):
It has `Response | ResponseInit` because `ResponseInit` no longer satisfies the type of `Response`.

![CleanShot 2024-12-26 at 15 37 15@2x](https://github.com/user-attachments/assets/3fbca5d2-9824-4cfd-bbc4-5fe4f9211f5f)


Code after:
}

interface SetHeadersOptions {
  append?: boolean
}

interface SetHeaders {
  (name: 'Content-Type', value?: BaseMime, options?: SetHeadersOptions): void
  (name: ResponseHeader, value?: string, options?: SetHeadersOptions): void
  (name: string, value?: string, options?: SetHeadersOptions): void
}

type ResponseHeadersInit =
  | [string, string][]
  | Record<'Content-Type', BaseMime>
  | Record<ResponseHeader, string>
  | Record<string, string>
  | Headers

interface ResponseInit<T extends StatusCode = StatusCode> {
  headers?: ResponseHeadersInit
  status?: T
  statusText?: string
}

type ResponseOrInit<T extends StatusCode = StatusCode> = ResponseInit<T> | Response

export const TEXT_PLAIN = 'text/plain; charset=UTF-8'

/**
 * Sets the headers of a response.
 *
 * @param headers - The Headers object to set the headers on.
 * @param map - A record of header key-value pairs to set.
 * @returns The updated Headers object.
 */
const setHeaders = (headers: Headers, map: Record<string, string> = {}) => {
  for (const key of Object.keys(map)) {
    headers.set(key, map[key])
  }
  return headers
}

export class Context<
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  E extends Env = any,
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  P extends string = any,
  I extends Input = {}
> {
  #rawRequest: Request
Repository: honojs/hono
File: src/context.test.ts (TypeScript)

Code before:
    c.header('X-Foo', 'Bar')
    c.header('X-Foo', 'Buzz', { append: true })
    const res = c.body('Hi', {
      headers: {
        'X-Message': 'Hi',
      },
    })
    expect(res.headers.get('X-Foo')).toBe('Bar, Buzz')
    expect(res.headers.get('X-Message')).toBe('Hi')
  })

  it('c.header() - append, c.html()', async () => {
    c.header('X-Foo', 'Bar', { append: true })
    const res = await c.html('<h1>This rendered fine</h1>')
    expect(res.headers.get('content-type')).toMatch(/^text\/html/)
  })

  it('c.header() - clear the header', async () => {
    c.header('X-Foo', 'Bar')
    c.header('X-Foo', undefined)
    c.header('X-Foo2', 'Bar')
    let res = c.body('Hi')
    expect(res.headers.get('X-Foo')).toBe(null)
    c.header('X-Foo2', undefined)
    res = c.res
    expect(res.headers.get('X-Foo2')).toBe(null)
  })

  it('c.header() - clear the header when append is true', async () => {
    c.header('X-Foo', 'Bar', { append: true })
    c.header('X-Foo', undefined)
    expect(c.res.headers.get('X-Foo')).toBe(null)
  })

  it('c.body() - multiple header', async () => {
    const res = c.body('Hi', 200, {
      'X-Foo': ['Bar', 'Buzz'],
    })
    const foo = res.headers.get('X-Foo')
    expect(foo).toBe('Bar, Buzz')
  })

  it('c.status()', async () => {
    c.status(201)
    const res = c.body('Hi')
    expect(res.status).toBe(201)
  })

  it('Complex pattern', async () => {
    c.status(404)
    const res = c.json({ hono: 'great app' })

Review comment (suggestion):
`c.body()` now has a type of `Response & TypedResponse`, so this change was made. The types are no longer compatible 

Code after:
    c.header('X-Foo', 'Bar')
    c.header('X-Foo', 'Buzz', { append: true })
    const res = c.body('Hi', {
      headers: {
        'X-Message': 'Hi',
      },
    })
    expect(res.headers.get('X-Foo')).toBe('Bar, Buzz')
    expect(res.headers.get('X-Message')).toBe('Hi')
  })

  it('c.header() - append, c.html()', async () => {
    c.header('X-Foo', 'Bar', { append: true })
    const res = await c.html('<h1>This rendered fine</h1>')
    expect(res.headers.get('content-type')).toMatch(/^text\/html/)
  })

  it('c.header() - clear the header', async () => {
    c.header('X-Foo', 'Bar')
    c.header('X-Foo', undefined)
    c.header('X-Foo2', 'Bar')
    const res = c.body('Hi')
    expect(res.headers.get('X-Foo')).toBe(null)
    c.header('X-Foo2', undefined)
    const res2 = c.body('Hi')
    expect(res2.headers.get('X-Foo2')).toBe(null)
  })

  it('c.header() - clear the header when append is true', async () => {
    c.header('X-Foo', 'Bar', { append: true })
    c.header('X-Foo', undefined)
    expect(c.res.headers.get('X-Foo')).toBe(null)
  })

  it('c.body() - multiple header', async () => {
    const res = c.body('Hi', 200, {
      'X-Foo': ['Bar', 'Buzz'],
    })
    const foo = res.headers.get('X-Foo')
    expect(foo).toBe('Bar, Buzz')
  })

  it('c.status()', async () => {
    c.status(201)
    const res = c.body('Hi')
    expect(res.status).toBe(201)
  })

  it('Complex pattern', async () => {
    c.status(404)
    const res = c.json({ hono: 'great app' })
Repository: honojs/hono
File: build/remove-private-fields.ts (TypeScript)

Code before:
import { cpus } from 'node:os'
import type { WorkerInput, WorkerOutput } from './remove-private-fields-worker'

const workers = Array.from({ length: Math.ceil(cpus().length / 2) }).map(
  () => new Worker(`${import.meta.dirname}/remove-private-fields-worker.ts`),
  { type: 'module' }
)
let workerIndex = 0
let taskId = 0

export async function removePrivateFields(file: string): Promise<string> {
  const currentTaskId = taskId++
  const worker = workers[workerIndex]
  workerIndex = (workerIndex + 1) % workers.length

  return new Promise<string>((resolve, reject) => {
    const abortController = new AbortController()
    worker.addEventListener(
      'message',
      ({ data: { type, value, taskId } }: { data: WorkerOutput }) => {
        if (taskId === currentTaskId) {
          if (type === 'success') {
            resolve(value)
          } else {
            reject(value)
          }

          abortController.abort()
        }
      },
      { signal: abortController.signal }
    )

Review comment (suggestion):
```suggestion
const workers = Array.from({ length: Math.ceil(cpus().length / 2) }).map(
  () => new Worker(`${import.meta.dirname}/remove-private-fields-worker.ts`, { type: 'module' })
)
```
I think it is typo.

Code after:
import { cpus } from 'node:os'
import type { WorkerInput, WorkerOutput } from './remove-private-fields-worker'

const workers = Array.from({ length: Math.ceil(cpus().length / 2) }).map(
  () => new Worker(`${import.meta.dirname}/remove-private-fields-worker.ts`, { type: 'module' })
)
let workerIndex = 0
let taskId = 0

export async function removePrivateFields(file: string): Promise<string> {
  const currentTaskId = taskId++
  const worker = workers[workerIndex]
  workerIndex = (workerIndex + 1) % workers.length

  return new Promise<string>((resolve, reject) => {
    const abortController = new AbortController()
    worker.addEventListener(
      'message',
      ({ data: { type, value, taskId } }: { data: WorkerOutput }) => {
        if (taskId === currentTaskId) {
          if (type === 'success') {
            resolve(value)
          } else {
            reject(value)
          }

          abortController.abort()
        }
      },
      { signal: abortController.signal }
    )
    worker.postMessage({ file, taskId: currentTaskId } satisfies WorkerInput)
Repository: honojs/hono
File: src/middleware/etag/digest.ts (TypeScript)

Code before:
const mergeBuffers = (buffer1: ArrayBuffer | undefined, buffer2: Uint8Array): Uint8Array => {
  if (!buffer1) {
    return buffer2
  }
  const merged = new Uint8Array(buffer1.byteLength + buffer2.byteLength)
  merged.set(new Uint8Array(buffer1), 0)
  merged.set(buffer2, buffer1.byteLength)
  return merged
}

export const generateDigest = async (
  stream: ReadableStream<Uint8Array> | null,
  generateDigest: (body: Uint8Array) => ArrayBuffer | Promise<ArrayBuffer>
): Promise<string | null> => {
  if (!stream || !crypto || !crypto.subtle) {
    return null
  }

  let result: ArrayBuffer | undefined = undefined

  const reader = stream.getReader()
  for (;;) {
    const { value, done } = await reader.read()
    if (done) {
      break
    }

    result = await generateDigest(mergeBuffers(result, value))
  }

  if (!result) {
    return null
  }

  return Array.prototype.map
    .call(new Uint8Array(result), (x) => x.toString(16).padStart(2, '0'))
    .join('')
}

Review comment (suggestion):
If the parameter has the same name `generateDigest` as the function, it may confuse people reading it later. It's fine to use a short name for internal use, so use a different name.

Code after:
const mergeBuffers = (buffer1: ArrayBuffer | undefined, buffer2: Uint8Array): Uint8Array => {
  if (!buffer1) {
    return buffer2
  }
  const merged = new Uint8Array(buffer1.byteLength + buffer2.byteLength)
  merged.set(new Uint8Array(buffer1), 0)
  merged.set(buffer2, buffer1.byteLength)
  return merged
}

export const generateDigest = async (
  stream: ReadableStream<Uint8Array> | null,
  generator: (body: Uint8Array) => ArrayBuffer | Promise<ArrayBuffer>
): Promise<string | null> => {
  if (!stream || !crypto || !crypto.subtle) {
    return null
  }

  let result: ArrayBuffer | undefined = undefined

  const reader = stream.getReader()
  for (;;) {
    const { value, done } = await reader.read()
    if (done) {
      break
    }

    result = await generator(mergeBuffers(result, value))
  }

  if (!result) {
    return null
  }

  return Array.prototype.map
    .call(new Uint8Array(result), (x) => x.toString(16).padStart(2, '0'))
    .join('')
}