ronantakizawa/codereview-bench huggingface dataset:
Repository: vitejs/vite
File: packages/vite/src/node/shortcuts.ts (TypeScript)

Code before:
      }

      return
    }

    const shortcut = shortcuts.find((shortcut) => shortcut.key === input)
    if (!shortcut || shortcut.action == null) return

    actionRunning = true
    await shortcut.action(server)
    actionRunning = false
  }

  const rl = readline.createInterface({ input: process.stdin })

  server._rl?.close()
  server._rl = rl

  rl.on('line', onInput)
  server.httpServer.on('close', () => rl.close())
}

const BASE_DEV_SHORTCUTS: CLIShortcut<ViteDevServer>[] = [
  {
    key: 'r',
    description: 'restart the server',
    async action(server) {
      await restartServerWithUrls(server)
    },
  },
  {
    key: 'u',
    description: 'show server url',
    action(server) {
      server.config.logger.info('')
      server.printUrls()
    },
  },
  {
    key: 'o',
    description: 'open in browser',
    action(server) {
      server.openBrowser()
    },
  },
  {
    key: 'c',
    description: 'clear console',
    action(server) {
      server.config.logger.clearScreen('error')
    },

Review comment (suggestion):
```suggestion
  server._rl ??= readline.createInterface({ input: process.stdin })
  const rl = server._rl
  rl.removeAllListeners('line')
```
I wonder if this is better as this way we can avoid the destruction / initialization cost.


Code after:
        server.config.logger.info(
          colors.dim('  press ') +
            colors.bold(`${shortcut.key} + enter`) +
            colors.dim(` to ${shortcut.description}`),
        )
      }

      return
    }

    const shortcut = shortcuts.find((shortcut) => shortcut.key === input)
    if (!shortcut || shortcut.action == null) return

    actionRunning = true
    await shortcut.action(server)
    actionRunning = false
  }

  if (!server._rl) {
    const rl = readline.createInterface({ input: process.stdin })
    server._rl = rl
    server.httpServer.on('close', () => rl.close())
  } else {
    server._rl.removeAllListeners('line')
  }

  server._rl.on('line', onInput)
}

const BASE_DEV_SHORTCUTS: CLIShortcut<ViteDevServer>[] = [
  {
    key: 'r',
    description: 'restart the server',
    async action(server) {
      await restartServerWithUrls(server)
    },
  },
  {
    key: 'u',
    description: 'show server url',
    action(server) {
      server.config.logger.info('')
      server.printUrls()
    },
  },
  {
    key: 'o',
    description: 'open in browser',
    action(server) {
      server.openBrowser()
    },
Repository: vitejs/vite
File: packages/vite/src/node/shortcuts.ts (TypeScript)

Code before:
}

export type CLIShortcut<Server = ViteDevServer | PreviewServer> = {
  key: string
  description: string
  action?(server: Server): void | Promise<void>
}

export function bindCLIShortcuts<Server extends ViteDevServer | PreviewServer>(
  server: Server,
  opts?: BindCLIShortcutsOptions<Server>,
  enabled: boolean = process.stdin.isTTY && !process.env.CI,
): void {
  if (!server.httpServer || !enabled) {
    return
  }

  const isDev = isDevServer(server)

  const customShortcuts: CLIShortcut<ViteDevServer | PreviewServer>[] =
    opts?.customShortcuts ?? []

  // Merge custom shortcuts from existing options
  // with new shortcuts taking priority
  for (const shortcut of server._shortcutsOptions?.customShortcuts ?? []) {
    if (!customShortcuts.find((s) => s.key === shortcut.key)) {
      customShortcuts.push(shortcut)
    }
  }

  server._shortcutsOptions = {
    ...opts,
    customShortcuts,
  }

  if (opts?.print) {
    server.config.logger.info(
      colors.dim(colors.green('  ➜')) +
        colors.dim('  press ') +
        colors.bold('h + enter') +
        colors.dim(' to show help'),
    )
  }

  const shortcuts = customShortcuts.concat(
    (isDev
      ? BASE_DEV_SHORTCUTS
      : BASE_PREVIEW_SHORTCUTS) as CLIShortcut<Server>[],
  )

  let actionRunning = false

Review comment (suggestion):
```suggestion
    if (!customShortcuts.some((s) => s.key === shortcut.key)) {
```

Code after:
}

export type CLIShortcut<Server = ViteDevServer | PreviewServer> = {
  key: string
  description: string
  action?(server: Server): void | Promise<void>
}

export function bindCLIShortcuts<Server extends ViteDevServer | PreviewServer>(
  server: Server,
  opts?: BindCLIShortcutsOptions<Server>,
  enabled: boolean = process.stdin.isTTY && !process.env.CI,
): void {
  if (!server.httpServer || !enabled) {
    return
  }

  const isDev = isDevServer(server)

  const customShortcuts: CLIShortcut<ViteDevServer | PreviewServer>[] =
    opts?.customShortcuts ?? []

  // Merge custom shortcuts from existing options
  // with new shortcuts taking priority
  for (const shortcut of server._shortcutsOptions?.customShortcuts ?? []) {
    if (!customShortcuts.some((s) => s.key === shortcut.key)) {
      customShortcuts.push(shortcut)
    }
  }

  server._shortcutsOptions = {
    ...opts,
    customShortcuts,
  }

  if (opts?.print) {
    server.config.logger.info(
      colors.dim(colors.green('  ➜')) +
        colors.dim('  press ') +
        colors.bold('h + enter') +
        colors.dim(' to show help'),
    )
  }

  const shortcuts = customShortcuts.concat(
    (isDev
      ? BASE_DEV_SHORTCUTS
      : BASE_PREVIEW_SHORTCUTS) as CLIShortcut<Server>[],
  )

  let actionRunning = false
Repository: vitejs/vite
File: packages/vite/src/node/server/index.ts (TypeScript)

Code before:
    // Keep the same connect instance so app.use(vite.middlewares) works
    // after a restart in middlewareMode (.route is always '/')
    middlewares.stack = newServer.middlewares.stack
    server.middlewares = middlewares

    // Rebind internal server variable so functions reference the user server
    newServer._setInternalServer(server)
  }

  const {
    logger,
    server: { port, middlewareMode },
  } = server.config
  if (!middlewareMode) {
    await server.listen(port, true)
  } else {
    await Promise.all(
      Object.values(server.environments).map((e) => e.listen(server)),
    )
  }
  logger.info('server restarted.', { timestamp: true })

  if (shortcutsOptions) {
    shortcutsOptions.print = false
    // Skip environment checks since shortcuts were bound before restart
    bindCLIShortcuts(server, shortcutsOptions, true)
  }
}

/**
 * Internal function to restart the Vite server and print URLs if changed
 */
export async function restartServerWithUrls(
  server: ViteDevServer,
): Promise<void> {
  if (server.config.server.middlewareMode) {
    await server.restart()
    return
  }

  const { port: prevPort, host: prevHost } = server.config.server
  const prevUrls = server.resolvedUrls

  await server.restart()

  const {
    logger,
    server: { port, host },
  } = server.config
  if (
    (port ?? DEFAULT_DEV_PORT) !== (prevPort ?? DEFAULT_DEV_PORT) ||

Review comment (suggestion):
```suggestion
    bindCLIShortcuts(
      server,
      shortcutsOptions,
      // Skip environment checks since shortcuts were bound before restart
      true,
    )
```
Just to make it clear which parameter this comment is referring to.


Code after:
    // Keep the same connect instance so app.use(vite.middlewares) works
    // after a restart in middlewareMode (.route is always '/')
    middlewares.stack = newServer.middlewares.stack
    server.middlewares = middlewares

    // Rebind internal server variable so functions reference the user server
    newServer._setInternalServer(server)
  }

  const {
    logger,
    server: { port, middlewareMode },
  } = server.config
  if (!middlewareMode) {
    await server.listen(port, true)
  } else {
    await Promise.all(
      Object.values(server.environments).map((e) => e.listen(server)),
    )
  }
  logger.info('server restarted.', { timestamp: true })

  if (shortcutsOptions) {
    shortcutsOptions.print = false
    bindCLIShortcuts(
      server,
      shortcutsOptions,
      // Skip environment checks since shortcuts were bound before restart
      true,
    )
  }
}

/**
 * Internal function to restart the Vite server and print URLs if changed
 */
export async function restartServerWithUrls(
  server: ViteDevServer,
): Promise<void> {
  if (server.config.server.middlewareMode) {
    await server.restart()
    return
  }

  const { port: prevPort, host: prevHost } = server.config.server
  const prevUrls = server.resolvedUrls

  await server.restart()

  const {
    logger,
Repository: vitejs/vite
File: packages/vite/src/node/env.ts (TypeScript)

Code before:
  }

  // check if there are actual env variables starting with VITE_*
  // these are typically provided inline and should be prioritized
  for (const key in process.env) {
    if (prefixes.some((prefix) => key.startsWith(prefix))) {
      env[key] = process.env[key] as string
    }
  }

  debug?.(`using resolved env: %O`, env)

  return env
}

export function resolveEnvPrefix({
  envPrefix = 'VITE_',
}: UserConfig): string[] {
  envPrefix = arraify(envPrefix)
  if (envPrefix.includes('')) {
    throw new Error(
      `envPrefix option contains value '', which could lead unexpected exposure of sensitive information.`,
    )
  }
  if (envPrefix.some((prefix) => /\s/.test(prefix))) {
    throw new Error(
      `envPrefix option contains values with whitespace, which could lead to unexpected behavior.`,
    )
  }
  return envPrefix
}


Review comment (suggestion):
```suggestion
  if (envPrefix.some((prefix) => prefix.includes(' '))) {
```

I think we can avoid a regex here

Code after:
    }
  }

  // check if there are actual env variables starting with VITE_*
  // these are typically provided inline and should be prioritized
  for (const key in process.env) {
    if (prefixes.some((prefix) => key.startsWith(prefix))) {
      env[key] = process.env[key] as string
    }
  }

  debug?.(`using resolved env: %O`, env)

  return env
}

export function resolveEnvPrefix({
  envPrefix = 'VITE_',
}: UserConfig): string[] {
  envPrefix = arraify(envPrefix)
  if (envPrefix.includes('')) {
    throw new Error(
      `envPrefix option contains value '', which could lead unexpected exposure of sensitive information.`,
    )
  }
  if (envPrefix.some((prefix) => /\s/.test(prefix))) {
    // eslint-disable-next-line no-console
    console.warn(
      colors.yellow(
        `[vite] Warning: envPrefix option contains values with whitespace, which does not work in practice.`,
      ),
    )
  }
  return envPrefix
}
Repository: vitejs/vite
File: packages/vite/src/node/env.ts (TypeScript)

Code before:
  // these are typically provided inline and should be prioritized
  for (const key in process.env) {
    if (prefixes.some((prefix) => key.startsWith(prefix))) {
      env[key] = process.env[key] as string
    }
  }

  debug?.(`using resolved env: %O`, env)

  return env
}

export function resolveEnvPrefix({
  envPrefix = 'VITE_',
}: UserConfig): string[] {
  envPrefix = arraify(envPrefix)
  if (envPrefix.includes('')) {
    throw new Error(
      `envPrefix option contains value '', which could lead unexpected exposure of sensitive information.`,
    )
  }
  if (envPrefix.some((prefix) => /\s/.test(prefix))) {
    // eslint-disable-next-line no-console
    console.warn(
      colors.yellow(
        `[vite] Warning: envPrefix option contains values with whitespace, which could lead to unexpected behavior.`,
      ),
    )
  }
  return envPrefix
}


Review comment (suggestion):
```suggestion
        `[vite] Warning: envPrefix option contains values with whitespace, which does not work in practice.`,
```

Maybe we can be a bit more accurate as we know it wont work in practice as env vars cant have whitespaces.

Code after:
  // these are typically provided inline and should be prioritized
  for (const key in process.env) {
    if (prefixes.some((prefix) => key.startsWith(prefix))) {
      env[key] = process.env[key] as string
    }
  }

  debug?.(`using resolved env: %O`, env)

  return env
}

export function resolveEnvPrefix({
  envPrefix = 'VITE_',
}: UserConfig): string[] {
  envPrefix = arraify(envPrefix)
  if (envPrefix.includes('')) {
    throw new Error(
      `envPrefix option contains value '', which could lead unexpected exposure of sensitive information.`,
    )
  }
  if (envPrefix.some((prefix) => /\s/.test(prefix))) {
    // eslint-disable-next-line no-console
    console.warn(
      colors.yellow(
        `[vite] Warning: envPrefix option contains values with whitespace, which does not work in practice.`,
      ),
    )
  }
  return envPrefix
}
Repository: vitejs/vite
File: packages/create-vite/src/index.ts (TypeScript)

Code before:
  --interactive / --no-interactive      force interactive / non-interactive mode

Available templates:
${yellow    ('vanilla-ts          vanilla'       )}
${green     ('vue-ts              vue'           )}
${cyan      ('react-ts            react'         )}
${cyan      ('react-compiler-ts   react-compiler')}
${cyan      ('react-swc-ts        react-swc'     )}
${magenta   ('preact-ts           preact'        )}
${redBright ('lit-ts              lit'           )}
${red       ('svelte-ts           svelte'        )}
${blue      ('solid-ts            solid'         )}
${blueBright('qwik-ts             qwik'          )}`

type ColorFunc = (str: string | number) => string
type Framework = {
  name: string
  display: string
  color: ColorFunc
  variants: FrameworkVariant[]
}
type FrameworkVariant = {
  name: string
  display: string
  description?: string
  link?: `https://${string}`
  color: ColorFunc
  customCommand?: string
}

const FRAMEWORKS: Framework[] = [
  {
    name: 'vanilla',
    display: 'Vanilla',
    color: yellow,
    variants: [
      {
        name: 'vanilla-ts',
        display: 'TypeScript',
        color: blue,
      },
      {
        name: 'vanilla',
        display: 'JavaScript',
        color: yellow,
      },
    ],
  },
  {
    name: 'vue',
    display: 'Vue',

Review comment (suggestion):
We think that the description doesn't help users much as the user won't be able to choose the templates only with that description and would need to open the website anyway. We also prefer to avoid maintaining the description.
As a conclusion, we think we should not add the description, but have the links.

Code after:
Available templates:
${yellow    ('vanilla-ts          vanilla'       )}
${green     ('vue-ts              vue'           )}
${cyan      ('react-ts            react'         )}
${cyan      ('react-compiler-ts   react-compiler')}
${cyan      ('react-swc-ts        react-swc'     )}
${magenta   ('preact-ts           preact'        )}
${redBright ('lit-ts              lit'           )}
${red       ('svelte-ts           svelte'        )}
${blue      ('solid-ts            solid'         )}
${blueBright('qwik-ts             qwik'          )}`

type ColorFunc = (str: string | number) => string
type Framework = {
  name: string
  display: string
  color: ColorFunc
  variants: FrameworkVariant[]
}
type FrameworkVariant = {
  name: string
  display: string
  link?: `https://${string}`
  color: ColorFunc
  customCommand?: string
}

const FRAMEWORKS: Framework[] = [
  {
    name: 'vanilla',
    display: 'Vanilla',
    color: yellow,
    variants: [
      {
        name: 'vanilla-ts',
        display: 'TypeScript',
        color: blue,
      },
      {
        name: 'vanilla',
        display: 'JavaScript',
        color: yellow,
      },
    ],
  },
  {
    name: 'vue',
    display: 'Vue',
    color: green,
    variants: [
      {
Repository: vitejs/vite
File: docs/.vitepress/theme/components/ScrimbaLink.vue (Vue)

Code before:
<script setup lang="ts">
defineProps<{
  href: string
  title: string
}>()
</script>

<template>
  <div class="scrimba">
    <span class="play-button">
      <span class="play-icon"></span>
    </span>
    <a :href="href" target="_blank" rel="sponsored noopener" :title="title">
      <slot>Watch an interactive lesson on Scrimba</slot>
    </a>
  </div>
</template>

<style scoped>
.scrimba {
  margin: 28px 0;
  background-color: var(--vp-c-bg-soft);
  padding: 1em 1.25em;
  border-radius: 8px;
  display: flex;
  align-items: center;
  gap: 12px;
}
.play-button {
  display: flex;
  align-items: center;
  justify-content: center;
  width: 26px;
  height: 26px;
  min-width: 26px;
  border-radius: 50%;
  background-color: var(--vp-c-brand-1);
}
.play-icon {
  width: 0;
  height: 0;
  margin-left: 2px;
  border-style: solid;
  border-width: 5px 0 5px 8px;
  border-color: transparent transparent transparent #fff;
}
.scrimba a {
  color: var(--vp-c-brand-1);

Review comment (suggestion):
```suggestion
  margin: 16px 0;
  color: var(--vp-custom-block-info-text);
  background-color: var(--vp-custom-block-info-bg);
  border: 1px solid var(--vp-custom-block-info-border);
  padding: 16px 20px;
```
I'd like to tweak the style a bit to match the `::: info` block.

Code after:
<script setup lang="ts">
defineProps<{
  href: string
  title: string
}>()
</script>

<template>
  <div class="scrimba">
    <span class="play-button">
      <span class="play-icon"></span>
    </span>
    <a :href="href" target="_blank" rel="sponsored noopener" :title="title">
      <slot>Watch an interactive lesson on Scrimba</slot>
    </a>
  </div>
</template>

<style scoped>
.scrimba {
  margin: 16px 0;
  color: var(--vp-custom-block-info-text);
  background-color: var(--vp-custom-block-info-bg);
  border: 1px solid var(--vp-custom-block-info-border);
  padding: 16px 20px;
  border-radius: 8px;
  display: flex;
  align-items: center;
  gap: 12px;
}
.play-button {
  display: flex;
  align-items: center;
  justify-content: center;
  width: 26px;
  height: 26px;
  min-width: 26px;
  border-radius: 50%;
  background-color: var(--vp-c-brand-1);
}
.play-icon {
  width: 0;
  height: 0;
  margin-left: 2px;
  border-style: solid;
  border-width: 5px 0 5px 8px;
  border-color: transparent transparent transparent #fff;
}
Repository: vitejs/vite
File: packages/vite/src/node/config.ts (TypeScript)

Code before:
import fs from 'node:fs'
import path from 'node:path'
import fsp from 'node:fs/promises'
import { pathToFileURL } from 'node:url'
import { inspect, promisify } from 'node:util'
import { performance } from 'node:perf_hooks'
import { createRequire } from 'node:module'
import crypto from 'node:crypto'
import colors from 'picocolors'
import picomatch from 'picomatch'
import {
  type NormalizedOutputOptions,
  type OutputChunk,
  type PluginContextMeta,
  type RolldownOptions,
  rolldown,
} from 'rolldown'
import type {
  DevToolsConfig,
  ResolvedDevToolsConfig,
} from '@vitejs/devtools/config'
import { normalizeDevToolsConfig } from '@vitejs/devtools/config'
import type { Alias, AliasOptions } from '#dep-types/alias'
import type { AnymatchFn } from '../types/anymatch'
import { withTrailingSlash } from '../shared/utils'
import {
  createImportMetaResolver,
  importMetaResolveWithCustomHookString,
} from '../module-runner/importMetaResolver'
import {
  CLIENT_ENTRY,
  DEFAULT_ASSETS_RE,
  DEFAULT_CLIENT_CONDITIONS,
  DEFAULT_CLIENT_MAIN_FIELDS,
  DEFAULT_CONFIG_FILES,
  DEFAULT_EXTENSIONS,
  DEFAULT_EXTERNAL_CONDITIONS,
  DEFAULT_PREVIEW_PORT,
  DEFAULT_SERVER_CONDITIONS,
  DEFAULT_SERVER_MAIN_FIELDS,
  ENV_ENTRY,
  FS_PREFIX,
} from './constants'
import { resolveEnvironmentPlugins } from './plugin'
import type {

Review comment (suggestion):
Not caused by this PR, but I noticed that we need to do something similar to https://github.com/vitejs/vite/blob/f1a47b36456298029f2a30cfef4e7318d780bad3/packages/vite/types/internal/lightningcssOptions.d.ts#L3-L4
otherwise an error would happen in the typecheck when skipLibCheck is disabled.


Code after:
import fs from 'node:fs'
import path from 'node:path'
import fsp from 'node:fs/promises'
import { pathToFileURL } from 'node:url'
import { inspect, promisify } from 'node:util'
import { performance } from 'node:perf_hooks'
import { createRequire } from 'node:module'
import crypto from 'node:crypto'
import colors from 'picocolors'
import picomatch from 'picomatch'
import {
  type NormalizedOutputOptions,
  type OutputChunk,
  type PluginContextMeta,
  type RolldownOptions,
  rolldown,
} from 'rolldown'
import type {
  DevToolsConfig,
  ResolvedDevToolsConfig,
} from '@vitejs/devtools/config'
import type { Alias, AliasOptions } from '#dep-types/alias'
import type { AnymatchFn } from '../types/anymatch'
import { withTrailingSlash } from '../shared/utils'
import {
  createImportMetaResolver,
  importMetaResolveWithCustomHookString,
} from '../module-runner/importMetaResolver'
import {
  CLIENT_ENTRY,
  DEFAULT_ASSETS_RE,
  DEFAULT_CLIENT_CONDITIONS,
  DEFAULT_CLIENT_MAIN_FIELDS,
  DEFAULT_CONFIG_FILES,
  DEFAULT_EXTENSIONS,
  DEFAULT_EXTERNAL_CONDITIONS,
  DEFAULT_PREVIEW_PORT,
  DEFAULT_SERVER_CONDITIONS,
  DEFAULT_SERVER_MAIN_FIELDS,
  ENV_ENTRY,
  FS_PREFIX,
} from './constants'
import { resolveEnvironmentPlugins } from './plugin'
import type {
  FalsyPlugin,
Repository: vitejs/vite
File: playground/proxy-bypass/vite.config.js (JavaScript)

Code before:
import { defineConfig } from 'vite'

const timeout = (ms) => new Promise((r) => setTimeout(r, ms))

export default defineConfig({
  server: {
    port: 9606,
    proxy: {
      '/nonExistentApp': {
        target: 'http://localhost:9607',
        bypass: () => {
          return false
        },
      },
      '/proxyError': {
        target: 'http://localhost:9608',
      },
      '/asyncResponse': {
        bypass: async (_, res) => {
          await timeout(4)
          res.writeHead(200, {
            'Content-Type': 'text/plain',
          })
          res.end('Hello after 4 ms (async timeout)')
          return '/asyncResponse'
        },
      },
      '/asyncThrowingError': {
        bypass: async () => {
          await timeout(4)
          throw new Error('bypass error')
        },
      },
    },
  },
  plugins: [
    {
      name: 'handle-error-in-preview',
      configurePreviewServer({ config, middlewares }) {
        return () => {
          middlewares.use((err, _req, res, _next) => {

Review comment (suggestion):
```suggestion
        target: 'http://foo.invalid',
```
@copilot 

Code after:
import { defineConfig } from 'vite'

const timeout = (ms) => new Promise((r) => setTimeout(r, ms))

export default defineConfig({
  server: {
    port: 9606,
    proxy: {
      '/nonExistentApp': {
        target: 'http://localhost:9607',
        bypass: () => {
          return false
        },
      },
      '/proxyError': {
        target: 'http://foo.invalid',
      },
      '/asyncResponse': {
        bypass: async (_, res) => {
          await timeout(4)
          res.writeHead(200, {
            'Content-Type': 'text/plain',
          })
          res.end('Hello after 4 ms (async timeout)')
          return '/asyncResponse'
        },
      },
      '/asyncThrowingError': {
        bypass: async () => {
          await timeout(4)
          throw new Error('bypass error')
        },
      },
    },
  },
  plugins: [
    {
      name: 'handle-error-in-preview',
      configurePreviewServer({ config, middlewares }) {
        return () => {
          middlewares.use((err, _req, res, _next) => {