ronantakizawa/codereview-bench huggingface dataset:
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/migrate-postcss.ts (TypeScript)

Code before:
const JS_CONFIG_FILE_LOCATIONS = [
  '.postcssrc.js',
  '.postcssrc.mjs',
  '.postcssrc.cjs',
  '.postcssrc.ts',
  '.postcssrc.mts',
  '.postcssrc.cts',
  'postcss.config.js',
  'postcss.config.mjs',
  'postcss.config.cjs',
  'postcss.config.ts',
  'postcss.config.mts',
  'postcss.config.cts',
]
async function detectJSConfigPath(base: string): Promise<null | string> {
  for (let file of JS_CONFIG_FILE_LOCATIONS) {
    let fullPath = path.resolve(base, file)
    try {
      await fs.access(fullPath)
      return fullPath
    } catch {}
  }
  return null
}

const JSON_CONFIG_FILE_LOCATIONS = [
  '.postcssrc',
  '.postcssrc.json',
  // yaml syntax is not supported
  // '.postcssrc.yml'
]
async function detectJSONConfigPath(base: string): Promise<null | string> {
  for (let file of JSON_CONFIG_FILE_LOCATIONS) {
    let fullPath = path.resolve(base, file)
    try {
      await fs.access(fullPath)
      return fullPath
    } catch {}
  }
  return null
}

async function isSimplePostCSSConfig(configPath: string): Promise<boolean> {
  let content = await fs.readFile(configPath, 'utf-8')
  return (
    content.includes('tailwindcss:') &&
    !(
      content.includes('require') ||
      // Adding a space at the end to not match `'postcss-import'`
      content.includes('import ')
    )

Review comment (bug):
We already know the absolute path, so returning this means that we don't have to re-compute the absolute path in other places where it is more error prone because we could resolve it against the wrong `base`.

Code after:
    didAddPostcssClient,
    didRemoveAutoprefixer,
    didRemovePostCSSImport,
  }
}

const JS_CONFIG_FILE_LOCATIONS = [
  '.postcssrc.js',
  '.postcssrc.mjs',
  '.postcssrc.cjs',
  '.postcssrc.ts',
  '.postcssrc.mts',
  '.postcssrc.cts',
  'postcss.config.js',
  'postcss.config.mjs',
  'postcss.config.cjs',
  'postcss.config.ts',
  'postcss.config.mts',
  'postcss.config.cts',
]
async function detectJSConfigPath(base: string): Promise<null | string> {
  for (let file of JS_CONFIG_FILE_LOCATIONS) {
    let fullPath = path.resolve(base, file)
    try {
      await fs.access(fullPath)
      return fullPath
    } catch {}
  }
  return null
}

const JSON_CONFIG_FILE_LOCATIONS = [
  '.postcssrc',
  '.postcssrc.json',
  // yaml syntax is not supported
  // '.postcssrc.yml'
]
async function detectJSONConfigPath(base: string): Promise<null | string> {
  for (let file of JSON_CONFIG_FILE_LOCATIONS) {
    let fullPath = path.resolve(base, file)
    try {
      await fs.access(fullPath)
      return fullPath
    } catch {}
  }
  return null
}

async function isSimplePostCSSConfig(configPath: string): Promise<boolean> {
  let content = await fs.readFile(configPath, 'utf-8')
  return (
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:
        {
          "dependencies": {
            "tailwindcss": "workspace:^",
            "@tailwindcss/upgrade": "workspace:^"
          }
        }
      `,
      'tailwind.config.ts': js` export default {} `,
      'src/index.html': html`
        <div class="underline"></div>
      `,
      'src/index.css': css`
        @tailwind base;
        @tailwind components;
        @tailwind utilities;
      `,
    },
  },
  async ({ exec }) => {
    let output = await exec('npx @tailwindcss/upgrade', {}, { ignoreStdErr: true }).catch((e) =>
      e.toString(),
    )

    output = stripVTControlCharacters(output)
      .replace(/tailwindcss v(.*)/g, 'tailwindcss') // Remove the version number from the error message
      .replace(/\\/g, '/') // Make Windows paths look like Unix paths

    expect(output).toMatch(
      /Tailwind CSS v.* found. The migration tool can only be run on v3 projects./,
    )
  },
)


Review comment (suggestion):
I don't think this is necessary because we use the `toMatch` with a regex.

```suggestion
```

Code after:
        <div class="blur blur-xs"></div>
        <div class="rounded rounded-sm"></div>
      </div>
      "
    `)
  },
)

test(
  'make suffix-less migrations safe (e.g.: `blur`, `rounded`, `shadow`)',
  {
    fs: {
      'package.json': json`
        {
          "dependencies": {
            "tailwindcss": "^3",
            "@tailwindcss/upgrade": "workspace:^"
          },
          "devDependencies": {
            "prettier-plugin-tailwindcss": "0.5.0"
          }
        }
      `,
      'tailwind.config.js': js`
        module.exports = {
          content: ['./*.{html,tsx}'],
        }
      `,
      'index.css': css`
        @tailwind base;
        @tailwind components;
        @tailwind utilities;
      `,
      'index.html': html`
        <div class="rounded blur shadow"></div>
      `,
      'example-component.tsx': ts`
        type Star = [
          x: number,
          y: number,
          dim?: boolean,
          blur?: boolean,
          rounded?: boolean,
          shadow?: boolean,
        ]

        function Star({ point: [cx, cy, dim, blur, rounded, shadow] }: { point: Star }) {
          return <svg class="rounded shadow blur" filter={blur ? 'url(…)' : undefined} />
        }
      `,
    },
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/utils/package-version.ts (TypeScript)

Code before:
import fs from 'node:fs/promises'
import { resolve } from 'node:path'

/**
 * Resolves the version string of an npm dependency installed in the based
 * directory.
 */
export async function getPackageVersion(pkg: string, base: string): Promise<string | null> {
  try {
    console.log('getPackageVersion', pkg, base)
    const packageJson = resolve(base, 'node_modules', pkg, 'package.json')
    const { version } = JSON.parse(await fs.readFile(packageJson, 'utf8'))
    return version
  } catch {
    return null
  }
}


Review comment (suggestion):
```suggestion
    let packageJson = resolve(base, 'node_modules', pkg, 'package.json')
    let { version } = JSON.parse(await fs.readFile(packageJson, 'utf8'))
```

Code after:
import fs from 'node:fs/promises'
import { createRequire } from 'node:module'

const localResolve = createRequire(import.meta.url).resolve

/**
 * Resolves the version string of an npm dependency installed in the based
 * directory.
 */
export async function getPackageVersion(pkg: string, base: string): Promise<string | null> {
  try {
    let packageJson = localResolve(`${pkg}/package.json`, { paths: [base] })
    let { version } = JSON.parse(await fs.readFile(packageJson, 'utf8'))
    return version
  } catch {
    return null
  }
}
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/utils/package-version.ts (TypeScript)

Code before:
import fs from 'node:fs/promises'
import { resolve } from 'node:path'

/**
 * Resolves the version string of an npm dependency installed in the based
 * directory.
 */
export async function getPackageVersion(pkg: string, base: string): Promise<string | null> {
  try {
    console.log('getPackageVersion', pkg, base)
    const packageJson = resolve(base, 'node_modules', pkg, 'package.json')
    const { version } = JSON.parse(await fs.readFile(packageJson, 'utf8'))
    return version
  } catch {
    return null
  }
}


Review comment (suggestion):
I'm not sure this will work in a monorepo where the `node_modules` can live in another spot. I wonder if a `require.resolve` would be better here 🤔

```suggestion
    let packageJson = require.resolve(`${pkg}/package.json`)
```

Code after:
import fs from 'node:fs/promises'
import { createRequire } from 'node:module'

const localResolve = createRequire(import.meta.url).resolve

/**
 * Resolves the version string of an npm dependency installed in the based
 * directory.
 */
export async function getPackageVersion(pkg: string, base: string): Promise<string | null> {
  try {
    let packageJson = localResolve(`${pkg}/package.json`, { paths: [base] })
    let { version } = JSON.parse(await fs.readFile(packageJson, 'utf8'))
    return version
  } catch {
    return null
  }
}
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-node/src/compile.ts (TypeScript)

Code before:

export async function __unstable__loadDesignSystem(css: string, { base }: { base: string }) {
  return ___unstable__loadDesignSystem(css, {
    base,
    async loadModule(id, base) {
      return loadModule(id, base, () => {})
    },
    async loadStylesheet(id, base) {
      return loadStylesheet(id, base, () => {})
    },
  })
}

export async function loadModule(id: string, base: string, onDependency: (path: string) => void) {
  if (id[0] !== '.') {
    let resolvedPath = await resolveJsId(id, base)
    if (!resolvedPath) {
      throw new Error(`Could not resolve '${id}' from '${base}'`)
    }

    if (typeof resolvedPath !== 'string') {
      return {
        base,
        module: resolvedPath,
      }
    }

    let module = await importModule(pathToFileURL(resolvedPath).href)
    return {
      base: dirname(resolvedPath),
      module: module.default ?? module,
    }
  }

  let resolvedPath = await resolveJsId(id, base)
  if (!resolvedPath) {
    throw new Error(`Could not resolve '${id}' from '${base}'`)
  }

  if (typeof resolvedPath !== 'string') {
    return {
      base,
      module: resolvedPath,
    }
  }

  let [module, moduleDependencies] = await Promise.all([
    importModule(pathToFileURL(resolvedPath).href + '?id=' + Date.now()),
    getModuleDependencies(resolvedPath),
  ])


Review comment (bug):
I know _why_ this is done this way but it feels super wrong. Can we have a `__tw_load` function or something instead?

Code after:

export async function __unstable__loadDesignSystem(css: string, { base }: { base: string }) {
  return ___unstable__loadDesignSystem(css, {
    base,
    async loadModule(id, base) {
      return loadModule(id, base, () => {})
    },
    async loadStylesheet(id, base) {
      return loadStylesheet(id, base, () => {})
    },
  })
}

export async function loadModule(id: string, base: string, onDependency: (path: string) => void) {
  if (id[0] !== '.') {
    let resolvedPath = await resolveJsId(id, base)
    if (!resolvedPath) {
      throw new Error(`Could not resolve '${id}' from '${base}'`)
    }

    let module = await importModule(pathToFileURL(resolvedPath).href)
    return {
      base: dirname(resolvedPath),
      module: module.default ?? module,
    }
  }

  let resolvedPath = await resolveJsId(id, base)
  if (!resolvedPath) {
    throw new Error(`Could not resolve '${id}' from '${base}'`)
  }

  let [module, moduleDependencies] = await Promise.all([
    importModule(pathToFileURL(resolvedPath).href + '?id=' + Date.now()),
    getModuleDependencies(resolvedPath),
  ])

  for (let file of moduleDependencies) {
    onDependency(file)
  }
  return {
    base: dirname(resolvedPath),
    module: module.default ?? module,
  }
}

async function loadStylesheet(id: string, base: string, onDependency: (path: string) => void) {
  let resolvedPath = await resolveCssId(id, base)
  if (!resolvedPath) throw new Error(`Could not resolve '${id}' from '${base}'`)

  onDependency(resolvedPath)
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/candidate.test.ts (TypeScript)

Code before:
      },
    ]
  `)
  expect(run(`tw:hover:flex`, { utilities, variants, prefix: 'tw' })).toMatchInlineSnapshot(`
    [
      {
        "important": false,
        "kind": "static",
        "raw": "tw:hover:flex",
        "root": "flex",
        "variants": [
          {
            "kind": "static",
            "root": "hover",
          },
        ],
      },
    ]
  `)
})

it.each([
  // Empty arbitrary value
  'bg-[]',
  'bg-()',
  //  Tricking the parser with a space is not allowed
  'bg-[_]',
  'bg-(_)',

  // Empty arbitrary value, with typehint
  'bg-[color:]',
  'bg-(color:)',
  //  Tricking the parser with a space is not allowed
  'bg-[color:_]',
  'bg-(color:_)',

  // Empty arbitrary modifier
  'bg-red-500/[]',
  'bg-red-500/()',
  //  Tricking the parser with a space is not allowed
  'bg-red-500/[_]',
  'bg-red-500/(_)',

  // Empty arbitrary modifier for arbitrary properties
  '[color:red]/[]',
  '[color:red]/()',
  //  Tricking the parser with a space is not allowed
  '[color:red]/[_]',
  '[color:red]/(_)',

  // Empty arbitrary value and modifier

Review comment (suggestion):
```suggestion
  // Tricking the parser with a space is not allowed
```

Code after:
      },
    ]
  `)
  expect(run(`tw:hover:flex`, { utilities, variants, prefix: 'tw' })).toMatchInlineSnapshot(`
    [
      {
        "important": false,
        "kind": "static",
        "raw": "tw:hover:flex",
        "root": "flex",
        "variants": [
          {
            "kind": "static",
            "root": "hover",
          },
        ],
      },
    ]
  `)
})

it.each([
  // Empty arbitrary value
  'bg-[]',
  'bg-()',
  // — Tricking the parser with a space is not allowed
  'bg-[_]',
  'bg-(_)',

  // Empty arbitrary value, with typehint
  'bg-[color:]',
  'bg-(color:)',
  // — Tricking the parser with a space is not allowed
  'bg-[color:_]',
  'bg-(color:_)',

  // Empty arbitrary modifier
  'bg-red-500/[]',
  'bg-red-500/()',
  // — Tricking the parser with a space is not allowed
  'bg-red-500/[_]',
  'bg-red-500/(_)',

  // Empty arbitrary modifier for arbitrary properties
  '[color:red]/[]',
  '[color:red]/()',
  // — Tricking the parser with a space is not allowed
  '[color:red]/[_]',
  '[color:red]/(_)',

  // Empty arbitrary value and modifier
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/candidate.test.ts (TypeScript)

Code before:
        "kind": "static",
        "raw": "tw:hover:flex",
        "root": "flex",
        "variants": [
          {
            "kind": "static",
            "root": "hover",
          },
        ],
      },
    ]
  `)
})

it.each([
  // Empty arbitrary value
  'bg-[]',
  'bg-()',
  //  Tricking the parser with a space is not allowed
  'bg-[_]',
  'bg-(_)',

  // Empty arbitrary value, with typehint
  'bg-[color:]',
  'bg-(color:)',
  //  Tricking the parser with a space is not allowed
  'bg-[color:_]',
  'bg-(color:_)',

  // Empty arbitrary modifier
  'bg-red-500/[]',
  'bg-red-500/()',
  //  Tricking the parser with a space is not allowed
  'bg-red-500/[_]',
  'bg-red-500/(_)',

  // Empty arbitrary modifier for arbitrary properties
  '[color:red]/[]',
  '[color:red]/()',
  //  Tricking the parser with a space is not allowed
  '[color:red]/[_]',
  '[color:red]/(_)',

  // Empty arbitrary value and modifier
  'bg-[]/[]',
  'bg-()/[]',
  'bg-[]/()',
  'bg-()/()',
  //  Tricking the parser with a space is not allowed
  'bg-[_]/[]',
  'bg-(_)/[]',

Review comment (suggestion):
```suggestion
  // Tricking the parser with a space is not allowed
```

Code after:
        "kind": "static",
        "raw": "tw:hover:flex",
        "root": "flex",
        "variants": [
          {
            "kind": "static",
            "root": "hover",
          },
        ],
      },
    ]
  `)
})

it.each([
  // Empty arbitrary value
  'bg-[]',
  'bg-()',
  // — Tricking the parser with a space is not allowed
  'bg-[_]',
  'bg-(_)',

  // Empty arbitrary value, with typehint
  'bg-[color:]',
  'bg-(color:)',
  // — Tricking the parser with a space is not allowed
  'bg-[color:_]',
  'bg-(color:_)',

  // Empty arbitrary modifier
  'bg-red-500/[]',
  'bg-red-500/()',
  // — Tricking the parser with a space is not allowed
  'bg-red-500/[_]',
  'bg-red-500/(_)',

  // Empty arbitrary modifier for arbitrary properties
  '[color:red]/[]',
  '[color:red]/()',
  // — Tricking the parser with a space is not allowed
  '[color:red]/[_]',
  '[color:red]/(_)',

  // Empty arbitrary value and modifier
  'bg-[]/[]',
  'bg-()/[]',
  'bg-[]/()',
  'bg-()/()',
  // — Tricking the parser with a space is not allowed
  'bg-[_]/[]',
  'bg-(_)/[]',
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/compat/config/types.ts (TypeScript)

Code before:
import type { Plugin, PluginWithConfig } from '../plugin-api'
import type { PluginUtils } from './resolve-config'

export type ResolvableTo<T> = T | ((utils: PluginUtils) => T)

export interface UserConfig {
  presets?: UserConfig[]
  theme?: ThemeConfig
  plugins?: Plugin[]
}

export type ThemeValue = ResolvableTo<Record<string, unknown>> | null | undefined
export type ResolvedThemeValue = Record<string, unknown> | null

export type ThemeConfig = Record<string, ThemeValue> & {
  extend?: Record<string, ThemeValue>
}

export interface ResolvedConfig {
  theme: Record<string, Record<string, unknown>>
  plugins: { plugin: PluginWithConfig; reference: boolean }[]
}

// Content support
type ContentFile = string | { raw: string; extension?: string }

export interface UserConfig {
  content?: ContentFile[] | { relative?: boolean; files: ContentFile[] }
}

type ResolvedContent = { base: string; pattern: string } | { raw: string; extension?: string }

export interface ResolvedContentConfig {
  files: ResolvedContent[]
}

export interface ResolvedConfig {
  content: ResolvedContentConfig
}

// Dark Mode support
type DarkModeStrategy =
  // No dark mode support
  | false

  // Use the `media` query strategy.

Review comment (bug):
This one feels wrong to change. In theory this type could be exposed to the user (it might be with the `config()` function actually — that works in v4 right?)

Code after:
import type { Plugin, PluginWithConfig } from '../plugin-api'
import type { PluginUtils } from './resolve-config'

export type ResolvableTo<T> = T | ((utils: PluginUtils) => T)

export interface UserConfig {
  presets?: UserConfig[]
  theme?: ThemeConfig
  plugins?: Plugin[]
}

export type ThemeValue = ResolvableTo<Record<string, unknown>> | null | undefined
export type ResolvedThemeValue = Record<string, unknown> | null

export type ThemeConfig = Record<string, ThemeValue> & {
  extend?: Record<string, ThemeValue>
}

export interface ResolvedConfig {
  theme: Record<string, Record<string, unknown>>
  plugins: PluginWithConfig[]
}

// Content support
type ContentFile = string | { raw: string; extension?: string }

export interface UserConfig {
  content?: ContentFile[] | { relative?: boolean; files: ContentFile[] }
}

type ResolvedContent = { base: string; pattern: string } | { raw: string; extension?: string }

export interface ResolvedContentConfig {
  files: ResolvedContent[]
}

export interface ResolvedConfig {
  content: ResolvedContentConfig
}

// Dark Mode support
type DarkModeStrategy =
  // No dark mode support
  | false

  // Use the `media` query strategy.
Repository: tailwindlabs/tailwindcss
File: crates/oxide/src/lib.rs (Rust)

Code before:

    match extension {
        Some("svelte") => Some(content.replace(" class:", " ")),
        _ => Some(content),
    }
}

#[tracing::instrument(skip_all)]
fn read_all_files(changed_content: Vec<ChangedContent>) -> Vec<Vec<u8>> {
    event!(
        tracing::Level::INFO,
        "Reading {:?} file(s)",
        changed_content.len()
    );

    changed_content
        .into_par_iter()
        .filter_map(read_changed_content)
        .collect()
}

#[tracing::instrument(skip_all)]
fn parse_all_blobs(blobs: Vec<Vec<u8>>) -> Vec<String> {
    let mut result: Vec<_> = blobs
        .par_iter()
        .flat_map(|blob| blob.par_split(|x| matches!(x, b'\n' | b'\r')))
        .map(|blob| Extractor::unique(blob, Default::default()))
        .reduce(Default::default, |mut a, b| {
            a.extend(b);
            a
        })
        .into_iter()
        .map(|s| {
            // SAFETY: When we parsed the candidates, we already guaranteed that the byte slices
            // are valid, therefore we don't have to re-check here when we want to convert it back
            // to a string.
            unsafe { String::from_utf8_unchecked(s.to_vec()) }
        })
        .collect();

    result.par_sort();
    result
}


Review comment (suggestion):
```suggestion
        .flat_map(|blob| blob.par_split(|x| matches!(x, b'\n')))
```

Might be enough to only look into `\n`, even for Windows, since otherwise every new line on Windows would emit a blank line as every line ends with both `\r` and `\n`

Code after:

    match extension {
        Some("svelte") => Some(content.replace(" class:", " ")),
        _ => Some(content),
    }
}

#[tracing::instrument(skip_all)]
fn read_all_files(changed_content: Vec<ChangedContent>) -> Vec<Vec<u8>> {
    event!(
        tracing::Level::INFO,
        "Reading {:?} file(s)",
        changed_content.len()
    );

    changed_content
        .into_par_iter()
        .filter_map(read_changed_content)
        .collect()
}

#[tracing::instrument(skip_all)]
fn parse_all_blobs(blobs: Vec<Vec<u8>>) -> Vec<String> {
    let mut result: Vec<_> = blobs
        .par_iter()
        .flat_map(|blob| blob.par_split(|x| matches!(x, b'\n')))
        .map(|blob| Extractor::unique(blob, Default::default()))
        .reduce(Default::default, |mut a, b| {
            a.extend(b);
            a
        })
        .into_iter()
        .map(|s| {
            // SAFETY: When we parsed the candidates, we already guaranteed that the byte slices
            // are valid, therefore we don't have to re-check here when we want to convert it back
            // to a string.
            unsafe { String::from_utf8_unchecked(s.to_vec()) }
        })
        .collect();

    result.par_sort();
    result
}
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/tsup.config.ts (TypeScript)

Code before:
import { defineConfig } from 'tsup'

export default defineConfig([
  {
    format: ['esm'],
    minify: true,
    dts: true,
    entry: {
      lib: 'src/index.ts',
      plugin: 'src/plugin.ts',
      colors: 'src/compat/colors.ts',
      'default-theme': 'src/compat/default-theme.ts',
      flattenColorPalette: 'src/compat/flatten-color-palette.ts',
    },
  },
  {
    format: ['cjs'],
    minify: true,
    dts: true,
    entry: {
      plugin: 'src/plugin.cts',
      lib: 'src/index.cts',
      colors: 'src/compat/colors.cts',
      'default-theme': 'src/compat/default-theme.cts',
      flattenColorPalette: 'src/compat/flatten-color-palette.cts',
    },
  },
])


Review comment (suggestion):
```suggestion
      'flatten-color-palette': 'src/compat/flatten-color-palette.ts',
```

Code after:
import { defineConfig } from 'tsup'

export default defineConfig([
  {
    format: ['esm'],
    minify: true,
    dts: true,
    entry: {
      lib: 'src/index.ts',
      plugin: 'src/plugin.ts',
      colors: 'src/compat/colors.ts',
      'default-theme': 'src/compat/default-theme.ts',
      'flatten-color-palette': 'src/compat/flatten-color-palette.ts',
    },
  },
  {
    format: ['cjs'],
    minify: true,
    dts: true,
    entry: {
      plugin: 'src/plugin.cts',
      lib: 'src/index.cts',
      colors: 'src/compat/colors.cts',
      'default-theme': 'src/compat/default-theme.cts',
      'flatten-color-palette': 'src/compat/flatten-color-palette.cts',
    },
  },
])