ronantakizawa/codereview-bench huggingface dataset:
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:

    const res = await app.fetch(req, {
      event,
      requestContext,
      lambdaContext,
    })

    return processor.createResult(event, res, { isContentTypeBinary })
  }
}

export abstract class EventProcessor<E extends LambdaEvent> {
  protected abstract getPath(event: E): string

  protected abstract getMethod(event: E): string

  protected abstract getQueryString(event: E): string

  protected abstract getHeaders(event: E): Headers

  protected abstract getCookies(event: E, headers: Headers): void

  protected abstract setCookiesToResult(result: APIGatewayProxyResult, cookies: string[]): void

  protected getHeaderValue(headers: E['headers'], key: string): string | undefined {
    let value = headers?.[key]

    if (Array.isArray(value)) {
      value = value[0]
    }

    return value
  }

  protected getDomainName(event: E): string | undefined {
    if (event.requestContext && 'domainName' in event.requestContext) {
      return event.requestContext.domainName
    }

    const hostFromHeaders = this.getHeaderValue(event.headers, 'host')

    if (hostFromHeaders) {
      return hostFromHeaders
    }

    const multiValueHeaders = 'multiValueHeaders' in event ? event.multiValueHeaders : {}
    const hostFromMultiValueHeaders = this.getHeaderValue(multiValueHeaders, 'host')

    return hostFromMultiValueHeaders
  }


Review comment (suggestion):
It's a matter of taste, but we often use the ternary operator to reduce the use of `let`. What do you think about writing this?

```ts
    const value = headers
      ? Array.isArray(headers[key])
        ? headers[key][0]
        : headers[key]
      : undefined
```

Code after:

    const res = await app.fetch(req, {
      event,
      requestContext,
      lambdaContext,
    })

    return processor.createResult(event, res, { isContentTypeBinary })
  }
}

export abstract class EventProcessor<E extends LambdaEvent> {
  protected abstract getPath(event: E): string

  protected abstract getMethod(event: E): string

  protected abstract getQueryString(event: E): string

  protected abstract getHeaders(event: E): Headers

  protected abstract getCookies(event: E, headers: Headers): void

  protected abstract setCookiesToResult(result: APIGatewayProxyResult, cookies: string[]): void

  protected getHeaderValue(headers: E['headers'], key: string): string | undefined {
    const value = headers
      ? Array.isArray(headers[key])
        ? headers[key][0]
        : headers[key]
      : undefined

    return value
  }

  protected getDomainName(event: E): string | undefined {
    if (event.requestContext && 'domainName' in event.requestContext) {
      return event.requestContext.domainName
    }

    const hostFromHeaders = this.getHeaderValue(event.headers, 'host')

    if (hostFromHeaders) {
      return hostFromHeaders
    }

    const multiValueHeaders = 'multiValueHeaders' in event ? event.multiValueHeaders : {}
    const hostFromMultiValueHeaders = this.getHeaderValue(multiValueHeaders, 'host')

    return hostFromMultiValueHeaders
  }
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:
  }

  protected setCookiesToResult(result: APIGatewayProxyResult, cookies: string[]): void {
    // when multi value headers is enabled
    if (result.multiValueHeaders) {
      result.multiValueHeaders['set-cookie'] = cookies
    } else {
      // otherwise serialize the set-cookie
      result.headers['set-cookie'] = cookies.join(', ')
    }
  }
}

const albProcessor: ALBProcessor = new ALBProcessor()

export class LatticeV2Processor extends EventProcessor<LatticeProxyEventV2> {
  protected getPath(event: LatticeProxyEventV2): string {
    return event.path
  }

  protected getMethod(event: LatticeProxyEventV2): string {
    return event.method
  }

  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  protected getQueryString(event: LatticeProxyEventV2): string {
    return ''
  }

  protected getHeaders(event: LatticeProxyEventV2): Headers {
    const headers = new Headers()

    if (event.headers) {
      for (const [k, values] of Object.entries(event.headers)) {
        if (values) {
          // avoid duplicating already set headers
          const foundK = headers.get(k)
          values.forEach((v) => {
            const sanitizedValue = sanitizeHeaderValue(v)
            return (
              (!foundK || !foundK.includes(sanitizedValue)) && headers.append(k, sanitizedValue)
            )
          })
        }
      }
    }

    return headers
  }

  protected getCookies(

Review comment (suggestion):
If you don't use `event`, why not remove it? Or do you have any reason for leaving it?

```ts
  protected getQueryString(): string {
    return ''
  }
```

Code after:
  }

  protected setCookiesToResult(result: APIGatewayProxyResult, cookies: string[]): void {
    // when multi value headers is enabled
    if (result.multiValueHeaders) {
      result.multiValueHeaders['set-cookie'] = cookies
    } else {
      // otherwise serialize the set-cookie
      result.headers['set-cookie'] = cookies.join(', ')
    }
  }
}

const albProcessor: ALBProcessor = new ALBProcessor()

export class LatticeV2Processor extends EventProcessor<LatticeProxyEventV2> {
  protected getPath(event: LatticeProxyEventV2): string {
    return event.path
  }

  protected getMethod(event: LatticeProxyEventV2): string {
    return event.method
  }

  protected getQueryString(): string {
    return ''
  }

  protected getHeaders(event: LatticeProxyEventV2): Headers {
    const headers = new Headers()

    if (event.headers) {
      for (const [k, values] of Object.entries(event.headers)) {
        if (values) {
          // avoid duplicating already set headers
          const foundK = headers.get(k)
          values.forEach((v) => {
            const sanitizedValue = sanitizeHeaderValue(v)
            return (
              (!foundK || !foundK.includes(sanitizedValue)) && headers.append(k, sanitizedValue)
            )
          })
        }
      }
    }

    return headers
  }

  protected getCookies(): void {
    // nop
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:
  protected getQueryString(event: LatticeProxyEventV2): string {
    return ''
  }

  protected getHeaders(event: LatticeProxyEventV2): Headers {
    const headers = new Headers()

    if (event.headers) {
      for (const [k, values] of Object.entries(event.headers)) {
        if (values) {
          // avoid duplicating already set headers
          const foundK = headers.get(k)
          values.forEach((v) => {
            const sanitizedValue = sanitizeHeaderValue(v)
            return (
              (!foundK || !foundK.includes(sanitizedValue)) && headers.append(k, sanitizedValue)
            )
          })
        }
      }
    }

    return headers
  }

  protected getCookies(
    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    event: LatticeProxyEventV2,
    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    headers: Headers
  ): void {
    // nop
  }

  protected setCookiesToResult(result: APIGatewayProxyResult, cookies: string[]): void {
    result.headers = {
      ...result.headers,
      'set-cookie': cookies.join(', '),
    }
  }
}

const latticeV2Processor: LatticeV2Processor = new LatticeV2Processor()

export const getProcessor = (event: LambdaEvent): EventProcessor<LambdaEvent> => {
  if (isProxyEventALB(event)) {
    return albProcessor
  }
  if (isProxyEventV2(event)) {
    return v2Processor
  }

Review comment (suggestion):
You can also remove the arguments.

```ts
  protected getCookies(): void {
    // nop
  }
```

Code after:
    return ''
  }

  protected getHeaders(event: LatticeProxyEventV2): Headers {
    const headers = new Headers()

    if (event.headers) {
      for (const [k, values] of Object.entries(event.headers)) {
        if (values) {
          // avoid duplicating already set headers
          const foundK = headers.get(k)
          values.forEach((v) => {
            const sanitizedValue = sanitizeHeaderValue(v)
            return (
              (!foundK || !foundK.includes(sanitizedValue)) && headers.append(k, sanitizedValue)
            )
          })
        }
      }
    }

    return headers
  }

  protected getCookies(): void {
    // nop
  }

  protected setCookiesToResult(result: APIGatewayProxyResult, cookies: string[]): void {
    result.headers = {
      ...result.headers,
      'set-cookie': cookies.join(', '),
    }
  }
}

const latticeV2Processor: LatticeV2Processor = new LatticeV2Processor()

export const getProcessor = (event: LambdaEvent): EventProcessor<LambdaEvent> => {
  if (isProxyEventALB(event)) {
    return albProcessor
  }
  if (isProxyEventV2(event)) {
    return v2Processor
  }
  if (isLatticeEventV2(event)) {
    return latticeV2Processor
  }

  return v1Processor
}
Repository: honojs/hono
File: src/router/reg-exp-router/prepared-router.ts (TypeScript)

Code before:
import type { ParamIndexMap, Result, Router } from '../../router'
import { METHOD_NAME_ALL } from '../../router'
import type { HandlerData, Matcher, MatcherMap, StaticMap } from './matcher'
import { match, buildAllMatchersKey, emptyParam } from './matcher'
import { RegExpRouter } from './router'

type RelocateMap = Record<string, [(number | string)[], ParamIndexMap | undefined][]>

export class PreparedRegExpRouter<T> implements Router<T> {
  name: string = 'PreparedRegExpRouter'
  #matchers: MatcherMap<T>
  #relocateMap: RelocateMap

  constructor(matchers: MatcherMap<T>, relocateMap: RelocateMap) {
    this.#matchers = matchers
    this.#relocateMap = relocateMap
  }

  add(method: string, path: string, handler: T) {
    const all = this.#matchers[METHOD_NAME_ALL] as Matcher<T>
    this.#matchers[method] ||= [
      all[0],
      all[1].map((list) => (Array.isArray(list) ? list.slice() : 0)) as HandlerData<T>[],
      Object.keys(all[2]).reduce((obj, key) => {
        obj[key] = [all[2][key][0].slice(), emptyParam] as Result<T>
        return obj
      }, {} as StaticMap<T>),
    ]

    if (path === '/*' || path === '*') {

Review comment (suggestion):
This is not related to this PR, but it uses only the type so that you can write like this:

```ts
import type { RegExpRouter } from './router'
```

Code after:
import type { ParamIndexMap, Result, Router } from '../../router'
import { METHOD_NAME_ALL } from '../../router'
import type { HandlerData, Matcher, MatcherMap, StaticMap } from './matcher'
import { match, emptyParam } from './matcher'
import { RegExpRouter } from './router'

type RelocateMap = Record<string, [(number | string)[], ParamIndexMap | undefined][]>

export class PreparedRegExpRouter<T> implements Router<T> {
  name: string = 'PreparedRegExpRouter'
  #matchers: MatcherMap<T>
  #relocateMap: RelocateMap

  constructor(matchers: MatcherMap<T>, relocateMap: RelocateMap) {
    this.#matchers = matchers
    this.#relocateMap = relocateMap
  }

  add(method: string, path: string, handler: T) {
    const all = this.#matchers[METHOD_NAME_ALL] as Matcher<T>
    this.#matchers[method] ||= [
      all[0],
      all[1].map((list) => (Array.isArray(list) ? list.slice() : 0)) as HandlerData<T>[],
      Object.keys(all[2]).reduce((obj, key) => {
        obj[key] = [all[2][key][0].slice(), emptyParam] as Result<T>
        return obj
      }, {} as StaticMap<T>),
    ]

    if (path === '/*' || path === '*') {
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:
  }
  if (isProxyEventV2(event)) {
    return v2Processor
  }
  return v1Processor
}

const isProxyEventALB = (event: LambdaEvent): event is ALBProxyEvent => {
  if (event.requestContext) {
    return Object.hasOwn(event.requestContext, 'elb')
  }
  return false
}

const isProxyEventV2 = (event: LambdaEvent): event is APIGatewayProxyEventV2 => {
  return Object.hasOwn(event, 'rawPath')
}

/**
 * Check if the given content type is binary.
 * This is a default function and may be overwritten by the user via `isContentTypeBinary` option in handler().
 * @param contentType The content type to check.
 * @returns True if the content type is binary, false otherwise.
 */
export const defaultIsContentTypeBinary = (contentType: string): boolean => {
  if (contentType.startsWith('application/vnd.openxmlformats-officedocument.')) return true
  return !/^(text\/(plain|html|css|javascript|csv).*|application\/(.*json|.*xml).*|image\/svg\+xml.*)$/.test(
    contentType
  )
}

export const isContentEncodingBinary = (contentEncoding: string | null) => {
  if (contentEncoding === null) {
    return false
  }
  return /^(gzip|deflate|compress|br)/.test(contentEncoding)
}


Review comment (suggestion):
Instead of adding an exception for a very particular format, can we possibly update the original regex on the next line to be something like this:

`^(text\/(plain|html|css|javascript|csv).*|application\/(.*json$|.*json\.|.*xml$|.*xml\.).*|image\/svg\+xml.*)$`

Added couple of additional checks, so that texts like "_application/iam+xml_", "_application/xml.blv.me_" are matched as xmls, but "_application/closedxmlcozwhynot_" is not treated as xml.

Code after:
  }
  if (isProxyEventV2(event)) {
    return v2Processor
  }
  return v1Processor
}

const isProxyEventALB = (event: LambdaEvent): event is ALBProxyEvent => {
  if (event.requestContext) {
    return Object.hasOwn(event.requestContext, 'elb')
  }
  return false
}

const isProxyEventV2 = (event: LambdaEvent): event is APIGatewayProxyEventV2 => {
  return Object.hasOwn(event, 'rawPath')
}

/**
 * Check if the given content type is binary.
 * This is a default function and may be overwritten by the user via `isContentTypeBinary` option in handler().
 * @param contentType The content type to check.
 * @returns True if the content type is binary, false otherwise.
 */
export const defaultIsContentTypeBinary = (contentType: string): boolean => {
  return !/^text\/(?:plain|html|css|javascript|csv)|(?:\/|\+)(?:json|xml)\s*(?:;|$)/.test(
    contentType
  )
}

export const isContentEncodingBinary = (contentEncoding: string | null) => {
  if (contentEncoding === null) {
    return false
  }
  return /^(gzip|deflate|compress|br)/.test(contentEncoding)
}
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:
  }
  if (isProxyEventV2(event)) {
    return v2Processor
  }
  return v1Processor
}

const isProxyEventALB = (event: LambdaEvent): event is ALBProxyEvent => {
  if (event.requestContext) {
    return Object.hasOwn(event.requestContext, 'elb')
  }
  return false
}

const isProxyEventV2 = (event: LambdaEvent): event is APIGatewayProxyEventV2 => {
  return Object.hasOwn(event, 'rawPath')
}

/**
 * Check if the given content type is binary.
 * This is a default function and may be overwritten by the user via `isContentTypeBinary` option in handler().
 * @param contentType The content type to check.
 * @returns True if the content type is binary, false otherwise.
 */
export const defaultIsContentTypeBinary = (contentType: string): boolean => {
  return !/^(text\/(plain|html|css|javascript|csv).*|application\/(.*json$|.*json\.|.*xml$|.*xml\.).*|image\/svg\+xml.*)$/.test(
    contentType
  )
}

export const isContentEncodingBinary = (contentEncoding: string | null) => {
  if (contentEncoding === null) {
    return false
  }
  return /^(gzip|deflate|compress|br)/.test(contentEncoding)
}


Review comment (suggestion):
Thank you for considering the changes.

As indicated in #4470, I believe we should make the following changes, including improvements to the current regular expression.

```suggestion
  return !/^text\/(?:plain|html|css|javascript|csv)|(?:\/|\+)(?:json|xml)(?:;|$)/test(
```

Code after:
  }
  if (isProxyEventV2(event)) {
    return v2Processor
  }
  return v1Processor
}

const isProxyEventALB = (event: LambdaEvent): event is ALBProxyEvent => {
  if (event.requestContext) {
    return Object.hasOwn(event.requestContext, 'elb')
  }
  return false
}

const isProxyEventV2 = (event: LambdaEvent): event is APIGatewayProxyEventV2 => {
  return Object.hasOwn(event, 'rawPath')
}

/**
 * Check if the given content type is binary.
 * This is a default function and may be overwritten by the user via `isContentTypeBinary` option in handler().
 * @param contentType The content type to check.
 * @returns True if the content type is binary, false otherwise.
 */
export const defaultIsContentTypeBinary = (contentType: string): boolean => {
  return !/^text\/(?:plain|html|css|javascript|csv)|(?:\/|\+)(?:json|xml)\s*(?:;|$)/.test(
    contentType
  )
}

export const isContentEncodingBinary = (contentEncoding: string | null) => {
  if (contentEncoding === null) {
    return false
  }
  return /^(gzip|deflate|compress|br)/.test(contentEncoding)
}
Repository: honojs/hono
File: src/middleware/timing/timing.ts (TypeScript)

Code before:
 * @param {Context} c - The context of the request.
 * @param {string} name - The name of the timer.
 * @param {Promise<T>} callable - The Promise to time.
 * @param {string} [description] - The description of the timer.
 * @param {number} [precision] - The precision of the timer value.
 *
 * @example
 * ```ts
 *   // Instead of this:
 *   const data = await db.findMany(...);
 *
 *   // do this:
 *   const data = await wrapTime(c, 'query', db.findMany(...));
 * ```
 * */
export async function wrapTime<T>(
  c: Context,
  name: string,
  callable: Promise<T>,
  description?: string,
  precision?: number
): Promise<T> {
  startTime(c, name, description)
  let result
  try {
    result = await callable
    return result
    // eslint-disable-next-line no-useless-catch
  } catch (e) {
    throw e
  } finally {
    endTime(c, name, precision)
  }
}


Review comment (suggestion):
I think you can write like this for this line:

```ts
return await callable
```

and, the `catch() {}` is really necessary?

If not, you can apply this patch:

```diff
diff --git a/src/middleware/timing/timing.ts b/src/middleware/timing/timing.ts
index 4c81285c..02fa87bb 100644
--- a/src/middleware/timing/timing.ts
+++ b/src/middleware/timing/timing.ts
@@ -247,13 +247,8 @@ export async function wrapTime<T>(
   precision?: number
 ): Promise<T> {
   startTime(c, name, description)
-  let result
   try {
-    result = await callable
-    return result
-    // eslint-disable-next-line no-useless-catch
-  } catch (e) {
-    throw e
+    return await callable
   } finally {
     endTime(c, name, precision)
   }
```

Code after:
 * @param {Context} c - The context of the request.
 * @param {string} name - The name of the timer.
 * @param {Promise<T>} callable - The Promise to time.
 * @param {string} [description] - The description of the timer.
 * @param {number} [precision] - The precision of the timer value.
 *
 * @example
 * ```ts
 *   // Instead of this:
 *   const data = await db.findMany(...);
 *
 *   // do this:
 *   const data = await wrapTime(c, 'query', db.findMany(...));
 * ```
 * */
export async function wrapTime<T>(
  c: Context,
  name: string,
  callable: Promise<T>,
  description?: string,
  precision?: number
): Promise<T> {
  startTime(c, name, description)
  try {
    return await callable
  } finally {
    endTime(c, name, precision)
  }
}
Repository: honojs/hono
File: src/middleware/context-storage/index.ts (TypeScript)

Code before:
 *   }
 * }
 *
 * const app = new Hono<Env>()
 *
 * app.use(contextStorage())
 *
 * app.use(async (c, next) => {
 *   c.set('message', 'Hono is hot!!)
 *   await next()
 * })
 *
 * app.get('/', async (c) => { c.text(getMessage()) })
 *
 * const getMessage = () => {
 *   return getContext<Env>().var.message
 * }
 * ```
 */
export const contextStorage = (): MiddlewareHandler => {
  return async function contextStorage(c, next) {
    await asyncLocalStorage.run(c, next)
  }
}

export const getContextIfAny = <E extends Env = Env>(): Context<E> | undefined => {
  return asyncLocalStorage.getStore() as Context<E> | undefined
}

export const getContext = <E extends Env = Env>(): Context<E> => {
  const context = getContextIfAny<E>()
  if (!context) {
    throw new Error('Context is not available')
  }
  return context
}


Review comment (suggestion):
```suggestion
export const tryGetContext = <E extends Env = Env>(): Context<E> | undefined => {
```

There’s nothing essential about this, but how about using this name?
AWS uses a similar name in the same context, and it improves the UX.

https://docs.aws.amazon.com/cdk/v2/guide/get-context-var.html#develop-context-retrieve



Code after:
 *   }
 * }
 *
 * const app = new Hono<Env>()
 *
 * app.use(contextStorage())
 *
 * app.use(async (c, next) => {
 *   c.set('message', 'Hono is hot!!)
 *   await next()
 * })
 *
 * app.get('/', async (c) => { c.text(getMessage()) })
 *
 * const getMessage = () => {
 *   return getContext<Env>().var.message
 * }
 * ```
 */
export const contextStorage = (): MiddlewareHandler => {
  return async function contextStorage(c, next) {
    await asyncLocalStorage.run(c, next)
  }
}

export const tryGetContext = <E extends Env = Env>(): Context<E> | undefined => {
  return asyncLocalStorage.getStore() as Context<E> | undefined
}

export const getContext = <E extends Env = Env>(): Context<E> => {
  const context = tryGetContext<E>()
  if (!context) {
    throw new Error('Context is not available')
  }
  return context
}
Repository: honojs/hono
File: src/helper/ssg/index.ts (TypeScript)

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

export * from './ssg'
export {
  X_HONO_DISABLE_SSG_HEADER_KEY,
  ssgParams,
  isSSGContext,
  disableSSG,
  onlySSG,
} from './middleware'
export { redirectPlugin } from './plugins'


Review comment (suggestion):
Since built-in plugins should only handle lightweight ones, I decided to manage multiple plugins in a single `plugins.ts` file. This should make updating exports slightly more convenient.


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

export * from './ssg'
export {
  X_HONO_DISABLE_SSG_HEADER_KEY,
  ssgParams,
  isSSGContext,
  disableSSG,
  onlySSG,
} from './middleware'
export { defaultPlugin, redirectPlugin } from './plugins'