ronantakizawa/codereview-bench huggingface dataset:
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:
          },
        }
      `,
      'src/index.html': html`
        <div class="bg-[--my-red]"></div>
      `,
      'src/index.css': css`
        @tailwind base;
        @tailwind components;
        @tailwind utilities;
      `,
    },
  },
  async ({ fs, exec }) => {
    await exec('npx @tailwindcss/upgrade')

    let packageJsonContent = await fs.read('package.json')
    let packageJson = JSON.parse(packageJsonContent)
    expect(packageJson.dependencies).toMatchObject({
      '@tailwindcss/postcss': expect.stringContaining('4.0.0'),
    })
  },
)

test(
  '`@tailwindcss/postcss` should be installed in dev dependencies when `tailwindcss` exists in dev dependencies',
  {
    fs: {
      'package.json': json`
        {
          "devDependencies": {
            "postcss": "^8",
            "tailwindcss": "^3",
            "@tailwindcss/upgrade": "workspace:^"
          }
        }
      `,
      'tailwind.config.js': js`
        /** @type {import('tailwindcss').Config} */
        module.exports = {
          content: ['./src/**/*.{html,js}'],
        }
      `,
      'postcss.config.js': js`
        module.exports = {
          plugins: {
            tailwindcss: {},
          },
        }
      `,
      'src/index.html': html`

Review comment (suggestion):
```suggestion
  '`@tailwindcss/postcss` should be installed in devDependencies when `tailwindcss` exists in dev dependencies',
```

Code after:
          },
        }
      `,
      'src/index.html': html`
        <div class="bg-[--my-red]"></div>
      `,
      'src/index.css': css`
        @tailwind base;
        @tailwind components;
        @tailwind utilities;
      `,
    },
  },
  async ({ fs, exec }) => {
    await exec('npx @tailwindcss/upgrade')

    let packageJsonContent = await fs.read('package.json')
    let packageJson = JSON.parse(packageJsonContent)
    expect(packageJson.dependencies).toMatchObject({
      '@tailwindcss/postcss': expect.stringContaining('4.0.0'),
    })
  },
)

test(
  '`@tailwindcss/postcss` should be installed in devDependencies when `tailwindcss` exists in dev dependencies',
  {
    fs: {
      'package.json': json`
        {
          "devDependencies": {
            "postcss": "^8",
            "tailwindcss": "^3",
            "@tailwindcss/upgrade": "workspace:^"
          }
        }
      `,
      'tailwind.config.js': js`
        /** @type {import('tailwindcss').Config} */
        module.exports = {
          content: ['./src/**/*.{html,js}'],
        }
      `,
      'postcss.config.js': js`
        module.exports = {
          plugins: {
            tailwindcss: {},
          },
        }
      `,
      'src/index.html': html`
Repository: tailwindlabs/tailwindcss
File: integrations/vite/other-transforms.test.ts (TypeScript)

Code before:
          ],
        })
      `,
      'index.html': html`
        <head>
          <link rel="stylesheet" href="./src/index.css" />
        </head>
        <body>
          <div class="foo [background-color:red]">Hello, world!</div>
        </body>
      `,
      'src/index.css': css`
        @import 'tailwindcss/theme' theme(reference);
        @import 'tailwindcss/utilities';

        .foo {
          color: red;
        }
      `,
    },
  }
}

for (let transformer of ['postcss', 'lightningcss'] as const) {
  describe(transformer, () => {
    test.only(`production build`, createSetup(transformer), async ({ fs, exec }) => {
      await exec('pnpm vite build')

      let files = await fs.glob('dist/**/*.css')
      expect(files).toHaveLength(1)
      let [filename] = files[0]

      await fs.expectFileToContain(filename, [
        css`
          .foo {
            color: blue;
          }
        `,
        // Running the transforms on utilities generated by Tailwind might change in the future
        dedent`
          .\[background-color\:red\] {
            background-color: blue;
          }
        `,
      ])
    })

    test(`dev mode`, createSetup(transformer), async ({ spawn, getFreePort, fs }) => {
      let port = await getFreePort()
      await spawn(`pnpm vite dev --port ${port}`)


Review comment (suggestion):
```suggestion
    test(`production build`, createSetup(transformer), async ({ fs, exec }) => {
```

Code after:
          ],
        })
      `,
      'index.html': html`
        <head>
          <link rel="stylesheet" href="./src/index.css" />
        </head>
        <body>
          <div class="foo [background-color:red]">Hello, world!</div>
        </body>
      `,
      'src/index.css': css`
        @import 'tailwindcss/theme' theme(reference);
        @import 'tailwindcss/utilities';

        .foo {
          color: red;
        }
      `,
    },
  }
}

for (let transformer of ['postcss', 'lightningcss'] as const) {
  describe(transformer, () => {
    test(`production build`, createSetup(transformer), async ({ fs, exec }) => {
      await exec('pnpm vite build')

      let files = await fs.glob('dist/**/*.css')
      expect(files).toHaveLength(1)
      let [filename] = files[0]

      await fs.expectFileToContain(filename, [
        css`
          .foo {
            color: blue;
          }
        `,
        // Running the transforms on utilities generated by Tailwind might change in the future
        dedent`
          .\[background-color\:red\] {
            background-color: blue;
          }
        `,
      ])
    })

    test(`dev mode`, createSetup(transformer), async ({ spawn, getFreePort, fs }) => {
      let port = await getFreePort()
      await spawn(`pnpm vite dev --port ${port}`)
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/index.ts (TypeScript)

Code before:
          continue
        }

        // Wrap `@keyframes` in `AtRoot` so they are hoisted out of `:root` when
        // printing.
        nodes.push(atRoot([keyframesRule]))
      }
    }
    firstThemeRule.nodes = nodes
  }

  // Replace the `@tailwind utilities` node with a conext since it prints
  // children directly.
  if (utilitiesNode) {
    let node = utilitiesNode as AstNode as Context
    node.kind = 'context'
    node.context = {}

    // Remove additional `@tailwind utilities` nodes
    walk(ast, (node, { replaceWith }) => {
      if (node.kind !== 'at-rule') return
      if (node.name !== '@tailwind') return
      if (!node.params.startsWith('utilities')) return

      replaceWith([])
    })
  }

  // Replace `@apply` rules with the actual utility classes.
  substituteAtApply(ast, designSystem)

  substituteFunctions(ast, designSystem.resolveThemeValue)

  // Remove `@utility`, we couldn't replace it before yet because we had to
  // handle the nested `@apply` at-rules first.
  walk(ast, (node, { replaceWith }) => {
    if (node.kind !== 'at-rule') return

    if (node.name === '@utility') {
      replaceWith([])
    }

    // The `@utility` has to be top-level, therefore we don't have to traverse
    // into nested trees.
    return WalkAction.Skip
  })

  return {
    designSystem,
    ast,
    globs,

Review comment (suggestion):
Can you think of any case where there are two `@tailwind utilities` nodes in the CSS file that is not "you screwed up your imports"? Wondering if we should just error on this case instead (since we already have a walk that will find `@tailwind utilities`.)

Code after:

      for (let keyframesRule of keyframesRules) {
        // Remove any keyframes that aren't used by an animation variable.
        let keyframesName = keyframesRule.params
        if (!animationParts.includes(keyframesName)) {
          continue
        }

        // Wrap `@keyframes` in `AtRoot` so they are hoisted out of `:root` when
        // printing.
        nodes.push(atRoot([keyframesRule]))
      }
    }
    firstThemeRule.nodes = nodes
  }

  // Replace the `@tailwind utilities` node with a context since it prints
  // children directly.
  if (utilitiesNode) {
    let node = utilitiesNode as AstNode as Context
    node.kind = 'context'
    node.context = {}
  }

  // Replace `@apply` rules with the actual utility classes.
  substituteAtApply(ast, designSystem)

  substituteFunctions(ast, designSystem.resolveThemeValue)

  // Remove `@utility`, we couldn't replace it before yet because we had to
  // handle the nested `@apply` at-rules first.
  walk(ast, (node, { replaceWith }) => {
    if (node.kind !== 'at-rule') return

    if (node.name === '@utility') {
      replaceWith([])
    }

    // The `@utility` has to be top-level, therefore we don't have to traverse
    // into nested trees.
    return WalkAction.Skip
  })

  return {
    designSystem,
    ast,
    globs,
    root,
    utilitiesNode,
  }
}
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.ts (TypeScript)

Code before:
      clone.base = path.dirname(clone.base)
    }

    return clone
  })
}

// Split a glob pattern into a `static` and `dynamic` part.
//
// Assumption: we assume that all globs are expanded, which means that the only
// dynamic parts are using `*`.
//
// E.g.:
//  Original input: `../project-b/**/*.{html,js}`
//  Expanded input: `../project-b/**/*.html` & `../project-b/**/*.js`
//  Split on first input: ("../project-b", "**/*.html")
//  Split on second input: ("../project-b", "**/*.js")
function splitPattern(pattern: string): [staticPart: string | null, dynamicPart: string | null] {
  // No dynamic parts, so we can just return the input as-is.
  if (!pattern.includes('*')) {
    return [pattern, null]
  }

  let lastSlashPosition: number | null = null

  for (let [i, c] of pattern.split('').entries()) {
    if (c === '/') {
      lastSlashPosition = i
    }

    if (c === '*' || c === '!') {
      break
    }
  }

  // Very first character is a `*`, therefore there is no static part, only a
  // dynamic part.
  if (lastSlashPosition === null) {
    return [null, pattern]
  }

  let staticPart = pattern.slice(0, lastSlashPosition).trim()
  let dynamicPart = pattern.slice(lastSlashPosition + 1).trim()

  return [staticPart || null, dynamicPart || null]
}


Review comment (suggestion):
Wonder if this is a bit easier to read if it's equivalent?

```suggestion
  for (let i = 0; i < pattern.length; i++) {
    let c = pattern[i];
```

Code after:
      clone.base = path.dirname(clone.base)
    }

    return clone
  })
}

// Split a glob pattern into a `static` and `dynamic` part.
//
// Assumption: we assume that all globs are expanded, which means that the only
// dynamic parts are using `*`.
//
// E.g.:
//  Original input: `../project-b/**/*.{html,js}`
//  Expanded input: `../project-b/**/*.html` & `../project-b/**/*.js`
//  Split on first input: ("../project-b", "**/*.html")
//  Split on second input: ("../project-b", "**/*.js")
function splitPattern(pattern: string): [staticPart: string | null, dynamicPart: string | null] {
  // No dynamic parts, so we can just return the input as-is.
  if (!pattern.includes('*')) {
    return [pattern, null]
  }

  let lastSlashPosition: number | null = null

  for (let i = 0; i < pattern.length; i++) {
    let c = pattern[i];
    if (c === '/') {
      lastSlashPosition = i
    }

    if (c === '*' || c === '!') {
      break
    }
  }

  // Very first character is a `*`, therefore there is no static part, only a
  // dynamic part.
  if (lastSlashPosition === null) {
    return [null, pattern]
  }

  let staticPart = pattern.slice(0, lastSlashPosition).trim()
  let dynamicPart = pattern.slice(lastSlashPosition + 1).trim()

  return [staticPart || null, dynamicPart || null]
}
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/template/codemods/important.ts (TypeScript)

Code before:
  designSystem: DesignSystem,
  _userConfig: Config,
  rawCandidate: string,
  location?: {
    contents: string
    start: number
    end: number
  },
): string {
  nextCandidate: for (let candidate of parseCandidate(rawCandidate, designSystem)) {
    if (candidate.important && candidate.raw[candidate.raw.length - 1] !== '!') {
      // The important migration is one of the most broad migrations with a high
      // potential of matching false positives since `!` is a valid character in
      // most programming languages. Since v4 is technically backward compatible
      // with v3 in that it can read `!` in the front of the utility too, we err
      // on the side of caution and only migrate candidates that we are certain
      // are inside of a string.
      if (location && !isSafeMigration(location)) {
        continue nextCandidate
      }
    }

    // The printCandidate function will already put the exclamation mark in the
    // right place, so we just need to mark this candidate as requiring a
    // migration.
    return printCandidate(designSystem, candidate)
  }

  return rawCandidate
}


Review comment (refactor):
Let's move this back inside the `if` condition, otherwise all candidates that are not ending with an exclamation mark are force-reprinted which does seem unexpected (e.g. if we have a serialize/deserialize issue it now affects all candidates) 

Code after:
  designSystem: DesignSystem,
  _userConfig: Config,
  rawCandidate: string,
  location?: {
    contents: string
    start: number
    end: number
  },
): string {
  nextCandidate: for (let candidate of parseCandidate(rawCandidate, designSystem)) {
    if (candidate.important && candidate.raw[candidate.raw.length - 1] !== '!') {
      // The important migration is one of the most broad migrations with a high
      // potential of matching false positives since `!` is a valid character in
      // most programming languages. Since v4 is technically backward compatible
      // with v3 in that it can read `!` in the front of the utility too, we err
      // on the side of caution and only migrate candidates that we are certain
      // are inside of a string.
      if (location && !isSafeMigration(location)) {
        continue nextCandidate
      }

      // The printCandidate function will already put the exclamation mark in
      // the right place, so we just need to mark this candidate as requiring a
      // migration.
      return printCandidate(designSystem, candidate)
    }
  }

  return rawCandidate
}
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:
          border-color: var(--color-gray-200, currentColor);
        }
      }

      --- ./src/base.css ---
      html {
        color: red;
      }

      --- ./src/components.css ---
      @utility foo {
        color: red;
      }

      --- ./src/mix.css ---
      @utility foo-mix {
        color: red;
      }

      @utility bar-mix {
        color: red;
      }

      html {
        color: blue;
      }

      --- ./src/utilities.css ---
      @utility bar {
        color: red;
      }
      "
    `)
  },
)

test(
  'migrates a simple postcss setup',
  {
    fs: {
      'package.json': json`
        {
          "dependencies": {
            "postcss": "^8",
            "postcss-cli": "^10",
            "postcss-import": "^16",
            "autoprefixer": "^10",
            "tailwindcss": "^3",
            "@tailwindcss/upgrade": "workspace:^"
          }
        }

Review comment (suggestion):
Something to think about: should this file be split into an import _with_ the `layer(…)` and one with the utilities to ensure that this `html {}` base CSS is in the correct spot. 🤔 

Code after:
        ::file-selector-button {
          border-color: var(--color-gray-200, currentColor);
        }
      }

      --- ./src/base.css ---
      html {
        color: red;
      }

      --- ./src/components.css ---
      @utility foo {
        color: red;
      }

      --- ./src/mix.css ---
      html {
        color: blue;
      }

      --- ./src/mix.utilities.css ---
      @utility foo-mix {
        color: red;
      }

      @utility bar-mix {
        color: red;
      }

      --- ./src/utilities.css ---
      @utility bar {
        color: red;
      }
      "
    `)
  },
)

test(
  'migrates a simple postcss setup',
  {
    fs: {
      'package.json': json`
        {
          "dependencies": {
            "postcss": "^8",
            "postcss-cli": "^10",
            "postcss-import": "^16",
            "autoprefixer": "^10",
            "tailwindcss": "^3",
            "@tailwindcss/upgrade": "workspace:^"
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:

      /*
        The default border color has changed to \`currentColor\` in Tailwind CSS v4,
        so we've added these compatibility styles to make sure everything still
        looks the same as it did with Tailwind CSS v3.

        If we ever want to remove these styles, we need to add an explicit border
        color utility to any element that depends on these defaults.
      */
      @layer base {
        *,
        ::after,
        ::before,
        ::backdrop,
        ::file-selector-button {
          border-color: var(--color-gray-200, currentColor);
        }
      }

      @layer base {
        html {
          color: red;
        }
      }

      --- ./src/components.css ---
      @import './typography.css' layer(components);

      --- ./src/components.utilities.css ---
      @utility foo {
        color: red;
      }

      --- ./src/tailwind-setup.css ---
      @import './base.css';
      @import './components.css';
      @import './utilities.css';

      @config '../tailwind.config.ts';

      --- ./src/tailwind-setup.utilities.css ---
      @import './components.utilities.css';
      @import './utilities.utilities.css'

      --- ./src/typography.css ---
      .typography {
        color: red;
      }

      --- ./src/utilities.css ---
      @import 'tailwindcss/utilities' layer(utilities);

Review comment (suggestion):
This has `layer(components)` because this is coming from the components file where nothing sits above it (in that file), but there is a `@layer components` and `@tailwind components` below it.

_if_ we want to look at as-if it was flattened, then it exists between `base` and `components`. This means that if this code existed in the same file we would've generated `layer(base)` instead. 🤔

Code after:
      /*
        The default border color has changed to \`currentColor\` in Tailwind CSS v4,
        so we've added these compatibility styles to make sure everything still
        looks the same as it did with Tailwind CSS v3.

        If we ever want to remove these styles, we need to add an explicit border
        color utility to any element that depends on these defaults.
      */
      @layer base {
        *,
        ::after,
        ::before,
        ::backdrop,
        ::file-selector-button {
          border-color: var(--color-gray-200, currentColor);
        }
      }

      @layer base {
        html {
          color: red;
        }
      }

      --- ./src/components.css ---
      @import './typography.css' layer(components);

      @utility foo {
        color: red;
      }

      --- ./src/tailwind-setup.css ---
      @import './base.css';
      @import './components.css';
      @import './utilities.css';

      @config '../tailwind.config.ts';

      --- ./src/typography.css ---
      .typography {
        color: red;
      }

      --- ./src/utilities.css ---
      @import 'tailwindcss/utilities' layer(utilities);

      @utility bar {
        color: red;
      }
      "
    `)
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:
      }

      @layer base {
        html {
          color: red;
        }
      }

      --- ./src/components.css ---
      @import './typography.css' layer(components);

      --- ./src/components.utilities.css ---
      @utility foo {
        color: red;
      }

      --- ./src/tailwind-setup.css ---
      @import './base.css';
      @import './components.css';
      @import './utilities.css';

      @config '../tailwind.config.ts';

      --- ./src/tailwind-setup.utilities.css ---
      @import './components.utilities.css';
      @import './utilities.utilities.css'

      --- ./src/typography.css ---
      .typography {
        color: red;
      }

      --- ./src/utilities.css ---
      @import 'tailwindcss/utilities' layer(utilities);

      --- ./src/utilities.utilities.css ---
      @utility bar {
        color: red;
      }
      "
    `)
  },
)

test(
  'injecting `@config` in the shared root (+ migrating), when a tailwind.config.{js,ts,…} is detected',
  {
    fs: {
      'package.json': json`
        {
          "dependencies": {

Review comment (suggestion):
Maybe we can avoid splitting up this file as `tailwind-setup.css` is also the Tailwind root. The result here does seem a bit unexpected 🤔 

Code after:

      @layer base {
        html {
          color: red;
        }
      }

      --- ./src/components.css ---
      @import './typography.css' layer(components);

      @utility foo {
        color: red;
      }

      --- ./src/tailwind-setup.css ---
      @import './base.css';
      @import './components.css';
      @import './utilities.css';

      @config '../tailwind.config.ts';

      --- ./src/typography.css ---
      .typography {
        color: red;
      }

      --- ./src/utilities.css ---
      @import 'tailwindcss/utilities' layer(utilities);

      @utility bar {
        color: red;
      }
      "
    `)
  },
)

test(
  'injecting `@config` in the shared root (+ migrating), when a tailwind.config.{js,ts,…} is detected',
  {
    fs: {
      'package.json': json`
        {
          "dependencies": {
            "@tailwindcss/upgrade": "workspace:^"
          }
        }
      `,
      'tailwind.config.ts': js`
        export default {
          content: ['./src/**/*.{html,js}'],
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:
        If we ever want to remove these styles, we need to add an explicit border
        color utility to any element that depends on these defaults.
      */
      @layer base {
        *,
        ::after,
        ::before,
        ::backdrop,
        ::file-selector-button {
          border-color: var(--color-gray-200, currentColor);
        }
      }

      @layer base {
        html {
          color: red;
        }
      }

      --- ./src/components.css ---
      @import './typography.css' layer(components);

      --- ./src/components.utilities.css ---
      @utility foo {
        color: red;
      }

      --- ./src/tailwind-setup.css ---
      @import './base.css';
      @import './components.css';
      @import './components.utilities.css';
      @import './utilities.css';
      @import './utilities.utilities.css';

      @theme {
        --color-my-red: red;
      }

      --- ./src/typography.css ---
      .typography {
        color: red;
      }

      --- ./src/utilities.css ---
      @import 'tailwindcss/utilities' layer(utilities);

      --- ./src/utilities.utilities.css ---
      @utility bar {
        color: red;
      }
      "

Review comment (refactor):
I think that technically since the only non `@utility` is an `@import` _with_ a `layer(…)` that this split shouldn't be required. But that feels like an optimization for a separate PR (if we even need that.).

Code after:
        *,
        ::after,
        ::before,
        ::backdrop,
        ::file-selector-button {
          border-color: var(--color-gray-200, currentColor);
        }
      }

      @layer base {
        html {
          color: red;
        }
      }

      --- ./src/components.css ---
      @import './typography.css' layer(components);

      @utility foo {
        color: red;
      }

      --- ./src/tailwind-setup.css ---
      @import './base.css';
      @import './components.css';
      @import './utilities.css';

      @theme {
        --color-my-red: red;
      }

      --- ./src/typography.css ---
      .typography {
        color: red;
      }

      --- ./src/utilities.css ---
      @import 'tailwindcss/utilities' layer(utilities);

      @utility bar {
        color: red;
      }
      "
    `)
  },
)

test(
  'relative imports without a relative path prefix are migrated to include a relative path prefix',
  {
    fs: {