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

Code before:
import type { Context, HeaderRecord } from '../../context'
import type { StatusCode } from '../../utils/http-status'
import { StreamingApi } from '../../utils/stream'

export const stream = (
  c: Context,
  cb: (stream: StreamingApi) => Promise<void>,
  arg?: StatusCode | ResponseInit,
  headers?: HeaderRecord
): Response => {
  const { readable, writable } = new TransformStream()
  const stream = new StreamingApi(writable)
  cb(stream).finally(() => stream.close())

  return typeof arg === 'number'
    ? c.newResponse(readable, arg, headers)
    : c.newResponse(readable, arg)
}

export { streamSSE } from './sse'
export { streamText } from './text'


Review comment (suggestion):
We can make it simpler:

```ts
export const stream = (
  c: Context,
  cb: (stream: StreamingApi) => Promise<void>,
): Response => {
  const { readable, writable } = new TransformStream()
  const stream = new StreamingApi(writable)
  cb(stream).finally(() => stream.close())
  return c.newResponse(readable)
}
```

Users can set the status and the headers with `c.status()` and `c.header(),` so I think it does not need `arg?` and `headers?`. `steamSSE()` also follows this approach.

Code after:
import type { Context } from '../../context'
import { StreamingApi } from '../../utils/stream'

export const stream = (c: Context, cb: (stream: StreamingApi) => Promise<void>): Response => {
  const { readable, writable } = new TransformStream()
  const stream = new StreamingApi(writable)
  cb(stream).finally(() => stream.close())
  return c.newResponse(readable)
}

export { streamSSE } from './sse'
export { streamText } from './text'
Repository: honojs/hono
File: src/helper/streaming/text.ts (TypeScript)

Code before:
import type { Context, HeaderRecord } from '../../context'
import { TEXT_PLAIN } from '../../context'
import type { StatusCode } from '../../utils/http-status'
import type { StreamingApi } from '../../utils/stream'
import { stream } from '.'

export const streamText = (
  c: Context,
  cb: (stream: StreamingApi) => Promise<void>,
  arg?: StatusCode | ResponseInit,
  headers?: HeaderRecord
): Response => {
  headers ??= {}
  c.header('content-type', TEXT_PLAIN)
  c.header('x-content-type-options', 'nosniff')
  c.header('transfer-encoding', 'chunked')
  return stream(c, cb, arg, headers)
}


Review comment (suggestion):
Same as above:

```ts
export const streamText = (
  c: Context,
  cb: (stream: StreamingApi) => Promise<void>,
): Response => {
  c.header('content-type', TEXT_PLAIN)
  c.header('x-content-type-options', 'nosniff')
  c.header('transfer-encoding', 'chunked')
  return stream(c, cb)
}
```

Code after:
import type { Context } from '../../context'
import { TEXT_PLAIN } from '../../context'
import type { StreamingApi } from '../../utils/stream'
import { stream } from '.'

export const streamText = (c: Context, cb: (stream: StreamingApi) => Promise<void>): Response => {
  c.header('Content-Type', TEXT_PLAIN)
  c.header('X-Content-Type-Options', 'nosniff')
  c.header('Transfer-Encoding', 'chunked')
  return stream(c, cb)
}
Repository: honojs/hono
File: src/hono-base.ts (TypeScript)

Code before:
import { compose } from './compose'
import { Context } from './context'
import type { ExecutionContext } from './context'
import { HTTPException } from './http-exception'
import { HonoRequest } from './request'
import type { Router } from './router'
import { METHOD_NAME_ALL, METHOD_NAME_ALL_LOWERCASE, METHODS } from './router'
import type {
  Env,
  ErrorHandler,
  H,
  HandlerInterface,
  MiddlewareHandler,
  MiddlewareHandlerInterface,
  Next,
  NotFoundHandler,
  OnHandlerInterface,
  MergePath,
  MergeSchemaPath,
  FetchEventLike,
  Schema,
  RouterRoute,
} from './types'
import { getPath, getPathNoStrict, getQueryStrings, mergePath } from './utils/url'

export const composedHandler = Symbol('composedHandler')

type Methods = typeof METHODS[number] | typeof METHOD_NAME_ALL_LOWERCASE

function defineDynamicClass(): {
  new <E extends Env = Env, S extends Schema = {}, BasePath extends string = '/'>(): {
    [M in Methods]: HandlerInterface<E, M, S, BasePath>
  } & {
    on: OnHandlerInterface<E, S, BasePath>
  } & {
    use: MiddlewareHandlerInterface<E, S, BasePath>
  }
} {
  return class {} as never
}

const notFoundHandler = (c: Context) => {
  return c.text('404 Not Found', 404)
}

const errorHandler = (err: Error, c: Context) => {
  if (err instanceof HTTPException) {
    return err.getResponse()
  }
  console.error(err)
  const message = 'Internal Server Error'

Review comment (suggestion):
Since `composedHandler` is a constant like `METHODS`, maybe `COMPOSED_HANDLER` would be better. What do you think?

```ts
export const COMPOSED_HANDLER = Symbol('composedHandler')
```

Code after:
import { compose } from './compose'
import { Context } from './context'
import type { ExecutionContext } from './context'
import { HTTPException } from './http-exception'
import { HonoRequest } from './request'
import type { Router } from './router'
import { METHOD_NAME_ALL, METHOD_NAME_ALL_LOWERCASE, METHODS } from './router'
import type {
  Env,
  ErrorHandler,
  H,
  HandlerInterface,
  MiddlewareHandler,
  MiddlewareHandlerInterface,
  Next,
  NotFoundHandler,
  OnHandlerInterface,
  MergePath,
  MergeSchemaPath,
  FetchEventLike,
  Schema,
  RouterRoute,
} from './types'
import { getPath, getPathNoStrict, getQueryStrings, mergePath } from './utils/url'

export const COMPOSED_HANDLER = Symbol('composedHandler')

type Methods = typeof METHODS[number] | typeof METHOD_NAME_ALL_LOWERCASE

function defineDynamicClass(): {
  new <E extends Env = Env, S extends Schema = {}, BasePath extends string = '/'>(): {
    [M in Methods]: HandlerInterface<E, M, S, BasePath>
  } & {
    on: OnHandlerInterface<E, S, BasePath>
  } & {
    use: MiddlewareHandlerInterface<E, S, BasePath>
  }
} {
  return class {} as never
}

const notFoundHandler = (c: Context) => {
  return c.text('404 Not Found', 404)
}

const errorHandler = (err: Error, c: Context) => {
  if (err instanceof HTTPException) {
    return err.getResponse()
  }
  console.error(err)
  const message = 'Internal Server Error'
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:
import { Buffer } from "node:buffer";
import * as path from 'node:path'
import { inspectRoutes } from '../../helper/dev/index.ts'
import type { Hono } from '../../hono.ts'
import type { Env, Schema } from '../../types.ts'

interface FileSystemModule {
  writeFile(path: string, data: string | Buffer): Promise<void>
  mkdir(path: string, options: { recursive: boolean }): Promise<void>
}

const generateFilePath = (routePath: string) => {
  const fileName = routePath === '/' ? 'index.html' : routePath + '.html'
  return path.join('./static', fileName)
}

export const generateHtmlMap = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>
): Promise<Map<string, string>> => {
  const htmlMap = new Map<string, string>()
  const baseURL = 'http://localhost'

  for (const route of inspectRoutes(app)) {
    if (route.isMiddleware) continue

    const url = new URL(route.path, baseURL).toString()
    const response = await app.fetch(new Request(url))
    const html = await response.text()

    htmlMap.set(route.path, html)

Review comment (suggestion):
Should be the following to make it compatible with `node:fs/promises`:

```ts
mkdir(path: string, options: { recursive: boolean }): Promise<void | string>
```

Code after:
import { Buffer } from "node:buffer";
import * as path from 'node:path'
import { inspectRoutes } from '../../helper/dev/index.ts'
import type { Hono } from '../../hono.ts'
import type { Env, Schema } from '../../types.ts'

export interface FileSystemModule {
  writeFile(path: string, data: string | Buffer): Promise<void>
  mkdir(path: string, options: { recursive: boolean }): Promise<void | string>
}

export interface ToSSGResult {
  success: boolean
  files?: string[]
  error?: Error
}

const generateFilePath = (routePath: string, outDir: string) => {
  const fileName = routePath === '/' ? 'index.html' : routePath + '.html'
  return path.join(outDir, fileName)
}

export const generateHtmlMap = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>
): Promise<Map<string, string>> => {
  const htmlMap = new Map<string, string>()
  const baseURL = 'http://localhost'

  for (const route of inspectRoutes(app)) {
    if (route.isMiddleware) continue
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:

    const url = new URL(route.path, baseURL).toString()
    const response = await app.fetch(new Request(url))
    const html = await response.text()

    htmlMap.set(route.path, html)
  }

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule
): Promise<void> => {
  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    console.log(`Written: ${filePath}`)
  }
}

export const toSsg = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>,
  fsModule: FileSystemModule
): Promise<void> => {
  const maps = await generateHtmlMap(app)
  await saveHtmlToLocal(maps, fsModule)
  console.log('Static site generation completed.')
}


Review comment (suggestion):
I prefer `toSSG` to `toSsg`. But I think either is fine.

Code after:
): Promise<Map<string, string>> => {
  const htmlMap = new Map<string, string>()
  const baseURL = 'http://localhost'

  for (const route of inspectRoutes(app)) {
    if (route.isMiddleware) continue

    const url = new URL(route.path, baseURL).toString()
    const response = await app.fetch(new Request(url))
    const html = await response.text()

    htmlMap.set(route.path, html)
  }

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule,
  outDir: string
): Promise<string[]> => {
  const files: string[] = []

  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath, outDir)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSSGInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (
    app: Hono<E, S, BasePath>,
    fsModule: FileSystemModule,
    options?: { dir?: string }
  ): Promise<ToSSGResult>
}

export interface ToSSGAdaptorInterface<
  E extends Env = Env,
  S extends Schema = {},
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule
): Promise<void> => {
  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    console.log(`Written: ${filePath}`)
  }
}

export const toSsg = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>,
  fsModule: FileSystemModule
): Promise<void> => {
  const maps = await generateHtmlMap(app)
  await saveHtmlToLocal(maps, fsModule)
  console.log('Static site generation completed.')
}


Review comment (suggestion):
`toSsg` should return a Result type or a list of output files. For example, we can pass the generated files to the configuration as follows:

```ts
files = await toSsg(app, fs, { dir: outDir })

return {
  build: {
    rollupOptions: {
      input: [...files],
    },
  },
}
```

Or you can use the Result type, which returns not only a list of files, but also whether the generation was successful, how many files were generated, etc.

Code after:

    htmlMap.set(route.path, html)
  }

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule,
  outDir: string
): Promise<string[]> => {
  const files: string[] = []

  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath, outDir)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSSGInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (
    app: Hono<E, S, BasePath>,
    fsModule: FileSystemModule,
    options?: { dir?: string }
  ): Promise<ToSSGResult>
}

export interface ToSSGAdaptorInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (app: Hono<E, S, BasePath>, options?: { dir?: string }): Promise<ToSSGResult>
}

export const toSSG: ToSSGInterface = async (app, fs, options) => {
  try {
    const outputDir = options?.dir ?? './static'
    const maps = await generateHtmlMap(app)
    const files = await saveHtmlToLocal(maps, fs, outputDir)
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule
): Promise<void> => {
  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    console.log(`Written: ${filePath}`)
  }
}

export const toSsg = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>,
  fsModule: FileSystemModule
): Promise<void> => {
  const maps = await generateHtmlMap(app)
  await saveHtmlToLocal(maps, fsModule)
  console.log('Static site generation completed.')
}


Review comment (suggestion):
Should be the following to make it compatible with `node:fs/promises`

```ts
Promise<string[]>
```

Code after:
    const url = new URL(route.path, baseURL).toString()
    const response = await app.fetch(new Request(url))
    const html = await response.text()

    htmlMap.set(route.path, html)
  }

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule,
  outDir: string
): Promise<string[]> => {
  const files: string[] = []

  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath, outDir)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSSGInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (
    app: Hono<E, S, BasePath>,
    fsModule: FileSystemModule,
    options?: { dir?: string }
  ): Promise<ToSSGResult>
}

export interface ToSSGAdaptorInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (app: Hono<E, S, BasePath>, options?: { dir?: string }): Promise<ToSSGResult>
}

export const toSSG: ToSSGInterface = async (app, fs, options) => {
  try {
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule
): Promise<void> => {
  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    console.log(`Written: ${filePath}`)
  }
}

export const toSsg = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>,
  fsModule: FileSystemModule
): Promise<void> => {
  const maps = await generateHtmlMap(app)
  await saveHtmlToLocal(maps, fsModule)
  console.log('Static site generation completed.')
}


Review comment (suggestion):
Should be the following to make it compatible with `node:fs/promises`:

```ts
Promise<string[]>
```

Code after:
    const url = new URL(route.path, baseURL).toString()
    const response = await app.fetch(new Request(url))
    const html = await response.text()

    htmlMap.set(route.path, html)
  }

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule,
  outDir: string
): Promise<string[]> => {
  const files: string[] = []

  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath, outDir)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSSGInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (
    app: Hono<E, S, BasePath>,
    fsModule: FileSystemModule,
    options?: { dir?: string }
  ): Promise<ToSSGResult>
}

export interface ToSSGAdaptorInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (app: Hono<E, S, BasePath>, options?: { dir?: string }): Promise<ToSSGResult>
}

export const toSSG: ToSSGInterface = async (app, fs, options) => {
  try {
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule
): Promise<void> => {
  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    console.log(`Written: ${filePath}`)
  }
}

export const toSsg = async <
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
>(
  app: Hono<E, S, BasePath>,
  fsModule: FileSystemModule
): Promise<void> => {
  const maps = await generateHtmlMap(app)
  await saveHtmlToLocal(maps, fsModule)
  console.log('Static site generation completed.')
}


Review comment (suggestion):
Should be the following to make it compatible with `node:fs/promises`.

```ts
Promise<string[]>
```

Code after:
    const url = new URL(route.path, baseURL).toString()
    const response = await app.fetch(new Request(url))
    const html = await response.text()

    htmlMap.set(route.path, html)
  }

  return htmlMap
}

export const saveHtmlToLocal = async (
  htmlMap: Map<string, string>,
  fsModule: FileSystemModule,
  outDir: string
): Promise<string[]> => {
  const files: string[] = []

  for (const [routePath, html] of htmlMap) {
    const filePath = generateFilePath(routePath, outDir)
    const dirPath = path.dirname(filePath)

    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSSGInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (
    app: Hono<E, S, BasePath>,
    fsModule: FileSystemModule,
    options?: { dir?: string }
  ): Promise<ToSSGResult>
}

export interface ToSSGAdaptorInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (app: Hono<E, S, BasePath>, options?: { dir?: string }): Promise<ToSSGResult>
}

export const toSSG: ToSSGInterface = async (app, fs, options) => {
  try {
Repository: honojs/hono
File: deno_dist/helper/ssg/index.ts (TypeScript)

Code before:
    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSsgInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (app: Hono<E, S, BasePath>, fsModule: FileSystemModule, options: { dir: string }): Promise<void>
}

export const toSSG = async (
  app: Hono,
  fsModule: FileSystemModule,
  options: { dir: string }
): Promise<ToSsgResult> => {
  try {
    const maps = await generateHtmlMap(app)
    const files = await saveHtmlToLocal(maps, fsModule, options.dir)

    console.log('Static site generation completed.')
    return { success: true, files }
  } catch (error) {
    const errorObj = error instanceof Error ? error : new Error(String(error))
    return { success: false, error: errorObj }
  }
}


Review comment (bug):
It would be helpful to have this message, but it would be better to allow users to change it at will. So, we can remove it. Maybe the user should just specify the message based on the result:

```ts
const result = await toSSG(app, fs)
if (result.success) {
  console.log('Static site generation completed!')
} else {
  console.log('Failed!', result.error.message())
}
```

@watany-dev 
How do you think about this?

Code after:
    await fsModule.mkdir(dirPath, { recursive: true })
    await fsModule.writeFile(filePath, html)
    files.push(filePath)
  }

  return files
}

export interface ToSSGInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (
    app: Hono<E, S, BasePath>,
    fsModule: FileSystemModule,
    options?: { dir?: string }
  ): Promise<ToSSGResult>
}

export interface ToSSGAdaptorInterface<
  E extends Env = Env,
  S extends Schema = {},
  BasePath extends string = '/'
> {
  (app: Hono<E, S, BasePath>, options?: { dir?: string }): Promise<ToSSGResult>
}

export const toSSG: ToSSGInterface = async (app, fs, options) => {
  try {
    const outputDir = options?.dir ?? './static'
    const maps = await generateHtmlMap(app)
    const files = await saveHtmlToLocal(maps, fs, outputDir)
    return { success: true, files }
  } catch (error) {
    const errorObj = error instanceof Error ? error : new Error(String(error))
    return { success: false, error: errorObj }
  }
}