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

Code before:
/* eslint-disable @typescript-eslint/no-explicit-any */
export type Expect<T extends true> = T
export type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2
  ? true
  : false
export type NotEqual<X, Y> = true extends Equal<X, Y> ? false : true

export type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (
  k: infer I
) => void
  ? I
  : never

export type JSONPrimitive = string | boolean | number | null | undefined
export type JSONArray = (JSONPrimitive | JSONObject | JSONArray)[]
export type JSONObject = { [key: string]: JSONPrimitive | JSONArray | JSONObject }
export type PrettyJSON<T> = T extends true
  ? boolean
  : T extends false
  ? boolean
  : T extends JSONPrimitive
  ? T
  : T extends JSONArray
  ? PrettyJSON<T[number]>
  : T extends JSONObject
  ? { [K in keyof T]: PrettyJSON<T[K]> }
  : never


Review comment (suggestion):
Top-level primitives and arrays can be valid JSONs. So the correct JSON type definition would be as follows IIUC.

```ts
export type JSONObject = JSONPrimitive | { [key: string]: JSONObject } | JSONObject[]
```

Code after:
/* eslint-disable @typescript-eslint/no-explicit-any */
export type Expect<T extends true> = T
export type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2
  ? true
  : false
export type NotEqual<X, Y> = true extends Equal<X, Y> ? false : true

export type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (
  k: infer I
) => void
  ? I
  : never

export type JSONPrimitive = string | boolean | number | null | undefined
export type JSONArray = (JSONPrimitive | JSONObject | JSONArray)[]
export type JSONObject = { [key: string]: JSONPrimitive | JSONArray | JSONObject }
export type JSONValue = JSONObject | JSONArray | JSONPrimitive

// `boolean` will be `true` or `false` because it's an alias of them.
// See: https://github.com/microsoft/TypeScript/issues/22596
// This type converts `true | false` to `boolean` that we expect.
type TrueAndFalseToBoolean<T> = T extends true ? boolean : T extends false ? boolean : T

export type PrettyJSON<T> = T extends JSONPrimitive
  ? TrueAndFalseToBoolean<T>
  : T extends JSONArray
  ? PrettyJSON<T[number]>
  : T extends JSONObject
  ? { [K in keyof T]: PrettyJSON<T[K]> }
  : never
Repository: honojs/hono
File: deno_dist/utils/types.ts (TypeScript)

Code before:
/* eslint-disable @typescript-eslint/no-explicit-any */
export type Expect<T extends true> = T
export type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2
  ? true
  : false
export type NotEqual<X, Y> = true extends Equal<X, Y> ? false : true

export type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (
  k: infer I
) => void
  ? I
  : never


Review comment (suggestion):
@yudai-nkt 

I added the comment and made `TrueAndFalseToBoolean` that is more descriptive. As noted here, `boolean` is distributed to `true | false`, so I'm fixing it. I'm sure there are other ways to do it, but this is how I did it.

Code after:
/* eslint-disable @typescript-eslint/no-explicit-any */
export type Expect<T extends true> = T
export type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2
  ? true
  : false
export type NotEqual<X, Y> = true extends Equal<X, Y> ? false : true

export type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (
  k: infer I
) => void
  ? I
  : never

export type JSONPrimitive = string | boolean | number | null | undefined
export type JSONArray = (JSONPrimitive | JSONObject | JSONArray)[]
export type JSONObject = { [key: string]: JSONPrimitive | JSONArray | JSONObject }
export type JSONValue = JSONObject | JSONArray | JSONPrimitive

// `boolean` will be `true` or `false` because it's an alias of them.
// See: https://github.com/microsoft/TypeScript/issues/22596
// This type converts `true | false` to `boolean` that we expect.
type TrueAndFalseToBoolean<T> = T extends true ? boolean : T extends false ? boolean : T

export type PrettyJSON<T> = T extends JSONPrimitive
  ? TrueAndFalseToBoolean<T>
  : T extends JSONArray
  ? PrettyJSON<T[number]>
  : T extends JSONObject
  ? { [K in keyof T]: PrettyJSON<T[K]> }
  : never
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:

  if (event.body) {
    requestInit.body = event.isBase64Encoded ? Buffer.from(event.body, 'base64') : event.body
  }

  return new Request(url, requestInit)
}

function extractQueryString(event: APIGatewayProxyEvent | APIGatewayProxyEventV2) {
  if (isProxyEventV2(event)) {
    return event.rawQueryString
  }

  return Object.entries(event.queryStringParameters)
    .filter(([, value]) => value)
    .map(([key, value]) => `${key}=${value}`)
    .join('&')
}

function isProxyEventV2(
  event: APIGatewayProxyEvent | APIGatewayProxyEventV2
): event is APIGatewayProxyEventV2 {
  return Object.prototype.hasOwnProperty.call(event, 'rawPath')
}

async function fromBufferTostring(res: Response) {
  const arrayBuffer = await res.arrayBuffer()
  return String.fromCharCode(...new Uint8Array(arrayBuffer))
}

async function fromReadableToString(res: Response) {
  const chunks = []
  const stream = res.body || new ReadableStream()

  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore: asking for asyncIterator
  for await (const chunk of stream) {
    chunks.push(chunk)
  }

  return Buffer.concat(chunks).toString('base64')
}


Review comment (suggestion):
You may use `bufferToString` in `utils/buffer.ts`.

https://github.com/honojs/hono/blob/27a966916165f4fbc24d58e2e8554e55c1cab8ed/src/utils/buffer.ts#L43

Code after:
  }

  return new Request(url, requestInit)
}

const extractQueryString = (event: APIGatewayProxyEvent | APIGatewayProxyEventV2) => {
  if (isProxyEventV2(event)) {
    return event.rawQueryString
  }

  return Object.entries(event.queryStringParameters || {})
    .filter(([, value]) => value)
    .map(([key, value]) => `${key}=${value}`)
    .join('&')
}

const isProxyEventV2 = (
  event: APIGatewayProxyEvent | APIGatewayProxyEventV2
): event is APIGatewayProxyEventV2 => {
  return Object.prototype.hasOwnProperty.call(event, 'rawPath')
}

const fromReadableToString = async (res: Response) => {
  const stream = res.body || new ReadableStream()
  const decoder = new TextDecoder()
  let string = ''

  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore: asking for asyncIterator
  for await (const chunk of stream) {
    string += decoder.decode(chunk)
  }

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

Code before:
    domainName: string
  }
}

interface APIGatewayProxyResult {
  statusCode: number
  body: string
  headers: Record<string, string>
  isBase64Encoded: boolean
}

/**
 * Accepts events from API Gateway/ELB(`APIGatewayProxyEvent`) and directly through Function Url(`APIGatewayProxyEventV2`)
 */
export const handle = (app: Hono) => {
  return async (
    event: APIGatewayProxyEvent | APIGatewayProxyEventV2
  ): Promise<APIGatewayProxyResult> => {
    const req = createRequest(event)
    const res = await app.fetch(req)

    return createResult(res)
  }
}

async function createResult(res: Response): Promise<APIGatewayProxyResult> {
  const isBase64Encoded = res.body instanceof ReadableStream
  const body = isBase64Encoded ? await fromReadableToString(res) : await fromBufferTostring(res)

  const result: APIGatewayProxyResult = {
    body,
    headers: {},
    statusCode: res.status,
    isBase64Encoded,
  }

  res.headers.forEach((value, key) => {
    result.headers[key] = value
  })

  return result
}

function createRequest(event: APIGatewayProxyEvent | APIGatewayProxyEventV2) {
  const queryString = extractQueryString(event)
  const urlPath = isProxyEventV2(event)
    ? `https://${event.requestContext.domainName}${event.rawPath}`
    : `https://${event.requestContext.domainName}${event.path}`

  const url = queryString ? `${urlPath}?${queryString}` : urlPath


Review comment (suggestion):
Please use arrow functions.

```ts
const createResult = (res: Response) => {} //...
```

Code after:
    domainName: string
  }
}

interface APIGatewayProxyResult {
  statusCode: number
  body: string
  headers: Record<string, string>
  isBase64Encoded: boolean
}

/**
 * Accepts events from API Gateway/ELB(`APIGatewayProxyEvent`) and directly through Function Url(`APIGatewayProxyEventV2`)
 */
export const handle = (app: Hono) => {
  return async (
    event: APIGatewayProxyEvent | APIGatewayProxyEventV2
  ): Promise<APIGatewayProxyResult> => {
    const req = createRequest(event)
    const res = await app.fetch(req)

    return createResult(res)
  }
}

const createResult = async (res: Response): Promise<APIGatewayProxyResult> => {
  const result: APIGatewayProxyResult = {
    body: await fromReadableToString(res),
    headers: {},
    statusCode: res.status,
    isBase64Encoded: true,
  }

  res.headers.forEach((value, key) => {
    result.headers[key] = value
  })

  return result
}

const createRequest = (event: APIGatewayProxyEvent | APIGatewayProxyEventV2) => {
  const queryString = extractQueryString(event)
  const urlPath = isProxyEventV2(event)
    ? `https://${event.requestContext.domainName}${event.rawPath}`
    : `https://${event.requestContext.domainName}${event.path}`

  const url = queryString ? `${urlPath}?${queryString}` : urlPath

  const headers = new Headers()
  for (const [k, v] of Object.entries(event.headers)) {
    if (v) headers.set(k, v)
Repository: honojs/hono
File: src/adapter/aws-lambda/handler.ts (TypeScript)

Code before:

  const url = queryString ? `${urlPath}?${queryString}` : urlPath

  const headers = new Headers()
  for (const [k, v] of Object.entries(event.headers)) {
    if (v) headers.set(k, v)
  }

  const requestInit: RequestInit = {
    headers,
    method: event.httpMethod,
  }

  if (event.body) {
    requestInit.body = event.isBase64Encoded ? Buffer.from(event.body, 'base64') : event.body
  }

  return new Request(url, requestInit)
}

function extractQueryString(event: APIGatewayProxyEvent | APIGatewayProxyEventV2) {
  if (isProxyEventV2(event)) {
    return event.rawQueryString
  }

  return Object.entries(event.queryStringParameters)
    .filter(([, value]) => value)
    .map(([key, value]) => `${key}=${value}`)
    .join('&')
}

function isProxyEventV2(
  event: APIGatewayProxyEvent | APIGatewayProxyEventV2
): event is APIGatewayProxyEventV2 {
  return Object.prototype.hasOwnProperty.call(event, 'rawPath')
}

async function fromBufferTostring(res: Response) {
  const arrayBuffer = await res.arrayBuffer()
  return String.fromCharCode(...new Uint8Array(arrayBuffer))
}

async function fromReadableToString(res: Response) {
  const chunks = []
  const stream = res.body || new ReadableStream()

  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore: asking for asyncIterator
  for await (const chunk of stream) {
    chunks.push(chunk)
  }

Review comment (suggestion):
Include what to do if `queryStringParameters` is falsy. When I tried this adapter, I got the following error.

<img width="723" alt="SS" src="https://user-images.githubusercontent.com/10682/227490297-fac09efe-e636-4cfe-937a-72213dc94b17.png">


Code after:
  const headers = new Headers()
  for (const [k, v] of Object.entries(event.headers)) {
    if (v) headers.set(k, v)
  }

  const requestInit: RequestInit = {
    headers,
    method: event.httpMethod,
  }

  if (event.body) {
    requestInit.body = event.isBase64Encoded ? atob(event.body) : event.body
  }

  return new Request(url, requestInit)
}

const extractQueryString = (event: APIGatewayProxyEvent | APIGatewayProxyEventV2) => {
  if (isProxyEventV2(event)) {
    return event.rawQueryString
  }

  return Object.entries(event.queryStringParameters || {})
    .filter(([, value]) => value)
    .map(([key, value]) => `${key}=${value}`)
    .join('&')
}

const isProxyEventV2 = (
  event: APIGatewayProxyEvent | APIGatewayProxyEventV2
): event is APIGatewayProxyEventV2 => {
  return Object.prototype.hasOwnProperty.call(event, 'rawPath')
}

const fromReadableToString = async (res: Response) => {
  const stream = res.body || new ReadableStream()
  const decoder = new TextDecoder()
  let string = ''

  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore: asking for asyncIterator
  for await (const chunk of stream) {
    string += decoder.decode(chunk)
  }

  return btoa(string)
}
Repository: honojs/hono
File: deno_dist/request.ts (TypeScript)

Code before:
            decoded[key] = /\%/.test(value) ? decodeURIComponent(value) : value
          }
        }

        return decoded
      }
    }
    return null
  }

  query(key: string): string | undefined
  query(): Record<string, string>
  query(key?: string) {
    return getQueryParam(this.url, key)
  }

  queries(key: string): string[] | undefined
  queries(): Record<string, string[]>
  queries(key?: string) {
    return getQueryParams(this.url, key)
  }

  header(name: string): string | undefined
  header(): Record<string, string>
  header(name?: string) {
    const headerData: Record<string, string> = {}
    this.raw.headers.forEach((value, key) => {
      headerData[key] = value
    })
    if (!name) {
      return headerData
    }
    return headerData[name.toLowerCase()] || undefined
  }

  cookie(key: string): string | undefined
  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(): Promise<BodyData> {
    return await parseBody(this.raw)

Review comment (suggestion):
Without this, I'd have to add `as string | unknown` to line 79, which is ugly. Ideally we'd turn on [`"noUncheckedIndexedAccess": true`](https://www.typescriptlang.org/tsconfig#noUncheckedIndexedAccess) in the tsconfig, but that makes _many_ things break so I consider it out of scope for this PR.

Code after:
            decoded[key] = /\%/.test(value) ? decodeURIComponent(value) : value
          }
        }

        return decoded
      }
    }
    return null
  }

  query(key: string): string | undefined
  query(): Record<string, string>
  query(key?: string) {
    return getQueryParam(this.url, key)
  }

  queries(key: string): string[] | undefined
  queries(): Record<string, string[]>
  queries(key?: string) {
    return getQueryParams(this.url, key)
  }

  header(name: string): string | undefined
  header(): Record<string, string>
  header(name?: string) {
    const headerData: Record<string, string | undefined> = {}
    this.raw.headers.forEach((value, key) => {
      headerData[key] = value
    })
    if (!name) {
      return headerData
    }
    return headerData[name.toLowerCase()]
  }

  cookie(key: string): string | undefined
  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(): Promise<BodyData> {
    return await parseBody(this.raw)
Repository: honojs/hono
File: deno_dist/middleware/etag/index.ts (TypeScript)

Code before:
import type { MiddlewareHandler } from '../../types.ts'
import { sha1 } from '../../utils/crypto.ts'

type ETagOptions = {
  weak: boolean
}

export const etag = (options: ETagOptions = { weak: false }): MiddlewareHandler => {
  return async (c, next) => {
    const ifNoneMatch = c.req.header('If-None-Match') || c.req.header('if-none-match')

    await next()

    const res = c.res as Response
    const clone = res.clone()
    const hash = await sha1(res.body || '')

    const etag = options.weak ? `W/"${hash}"` : `"${hash}"`

    if (ifNoneMatch && ifNoneMatch === etag) {
      await clone.blob() // Force using body
      c.res = new Response(null, {
        status: 304,
        statusText: 'Not Modified',
        headers: {
          ETag: etag,
        },
      })
      c.res.headers.delete('Content-Length')
    } else {
      c.res = new Response(clone.body, clone)
      c.res.headers.set('ETag', etag)
    }
  }
}


Review comment (suggestion):
After some real-world testing, I think this should be

```ts
c.res = undisturbedRes
```

There are some environments where this is causing the non-304 responses to have empty body.

Code after:
    const res = c.res as Response
    let undisturbedRes = res
    let etag = res.headers.get('ETag')

    if (!etag) {
      undisturbedRes = res.clone()
      const hash = await sha1(res.body || '')
      etag = weak ? `W/"${hash}"` : `"${hash}"`
    }

    if (etagMatches(etag, ifNoneMatch)) {
      await undisturbedRes.blob() // Force using body
      c.res = new Response(null, {
        status: 304,
        statusText: 'Not Modified',
        headers: {
          ETag: etag,
        },
      })
      c.res.headers.forEach((_, key) => {
        if (retainedHeaders.indexOf(key.toLowerCase()) === -1) {
          c.res.headers.delete(key)
        }
      })
    } else {
      c.res = new Response(undisturbedRes.body, undisturbedRes)
      c.res.headers.set('ETag', etag)
    }
  }
}
Repository: honojs/hono
File: deno_dist/adapter/deno/serve-static.ts (TypeScript)

Code before:
import type { Context } from '../../context.ts'
import type { Next } from '../../types.ts'
import { getFilePath } from '../../utils/filepath.ts'
import { getMimeType } from '../../utils/mime.ts'

export type ServeStaticOptions = {
  root?: string
  path?: string
  rewriteRequestPath?: (path: string) => string
}

const DEFAULT_DOCUMENT = 'index.html'

export const serveStatic = (options: ServeStaticOptions = { root: '' }) => {
  return async (c: Context, next: Next) => {
    // Do nothing if Response is already set
    if (c.finalized) {
      await next()
      return
    }

    const url = new URL(c.req.url)
    const filename = options.path ?? decodeURI(url.pathname)
    let path = getFilePath({
      filename: options.rewriteRequestPath ? options.rewriteRequestPath(filename) : filename,
      root: options.root,
      defaultDocument: DEFAULT_DOCUMENT,
    })

    path = `./${path}`

    let content


Review comment (suggestion):
If don't ignore the types like this, it's difficult to pass the `tsc` because we need type definitions for Deno.

Code after:
import type { Context } from '../../context.ts'
import type { Next } from '../../types.ts'
import { getFilePath } from '../../utils/filepath.ts'
import { getMimeType } from '../../utils/mime.ts'

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const { readFile } = Deno

export type ServeStaticOptions = {
  root?: string
  path?: string
  rewriteRequestPath?: (path: string) => string
}

const DEFAULT_DOCUMENT = 'index.html'

export const serveStatic = (options: ServeStaticOptions = { root: '' }) => {
  return async (c: Context, next: Next) => {
    // Do nothing if Response is already set
    if (c.finalized) {
      await next()
      return
    }

    const url = new URL(c.req.url)
    const filename = options.path ?? decodeURI(url.pathname)
    let path = getFilePath({
      filename: options.rewriteRequestPath ? options.rewriteRequestPath(filename) : filename,
      root: options.root,
      defaultDocument: DEFAULT_DOCUMENT,
    })
Repository: honojs/hono
File: deno_dist/client/types.ts (TypeScript)

Code before:

type ClientRequest<S extends Data> = {
  [M in keyof S]: S[M] extends { input: infer R; output: infer O }
    ? RemoveBlankRecord<R> extends never
      ? (args?: {}, options?: ClientRequestOptions) => Promise<ClientResponse<O>>
      : (args: R, options?: ClientRequestOptions) => Promise<ClientResponse<O>>
    : never
}

export interface ClientResponse<T> {
  ok: boolean
  status: number
  statusText: string
  headers: Headers
  url: string
  redirect(url: string, status: number): Response
  clone(): Response
  json(): Promise<T>
  text(): Promise<string>
  blob(): Promise<Blob>
  formData(): Promise<FormData>
  arrayBuffer(): Promise<ArrayBuffer>
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export interface Response extends ClientResponse<any> {}

export type Fetch<T> = (
  args?: InferRequestType<T>,
  opt?: ClientRequestOptions
) => Promise<ClientResponse<InferResponseType<T>>>

export type InferResponseType<T> = T extends (
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  args: any | undefined
) => Promise<ClientResponse<infer O>>
  ? O
  : never

export type InferRequestType<T> = T extends (args: infer R) => Promise<ClientResponse<unknown>>
  ? NonNullable<R>
  : never

type PathToChain<
  Path extends string,
  E extends Endpoint,
  Original extends string = ''
> = Path extends `/${infer P}`
  ? PathToChain<P, E, Path>
  : Path extends `${infer P}/${infer R}`
  ? { [K in P]: PathToChain<R, E, Original> }

Review comment (suggestion):
perhaps you could use `unknown` instead of `any` here? like:

`export interface Response extends ClientResponse<unknown> {}`

Code after:

type ClientRequest<S extends Data> = {
  [M in keyof S]: S[M] extends { input: infer R; output: infer O }
    ? RemoveBlankRecord<R> extends never
      ? (args?: {}, options?: ClientRequestOptions) => Promise<ClientResponse<O>>
      : (args: R, options?: ClientRequestOptions) => Promise<ClientResponse<O>>
    : never
}

export interface ClientResponse<T> {
  ok: boolean
  status: number
  statusText: string
  headers: Headers
  url: string
  redirect(url: string, status: number): Response
  clone(): Response
  json(): Promise<T>
  text(): Promise<string>
  blob(): Promise<Blob>
  formData(): Promise<FormData>
  arrayBuffer(): Promise<ArrayBuffer>
}

export interface Response extends ClientResponse<unknown> {}

export type Fetch<T> = (
  args?: InferRequestType<T>,
  opt?: ClientRequestOptions
) => Promise<ClientResponse<InferResponseType<T>>>

export type InferResponseType<T> = T extends (
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  args: any | undefined
) => Promise<ClientResponse<infer O>>
  ? O
  : never

export type InferRequestType<T> = T extends (args: infer R) => Promise<ClientResponse<unknown>>
  ? NonNullable<R>
  : never

type PathToChain<
  Path extends string,
  E extends Endpoint,
  Original extends string = ''
> = Path extends `/${infer P}`
  ? PathToChain<P, E, Path>
  : Path extends `${infer P}/${infer R}`
  ? { [K in P]: PathToChain<R, E, Original> }
  : {