Fix template migration issues (#14600)

This PR fixes two issues we found when testing the candidate codemodes: 1. Sometimes, core would emit the same candidate twice. This would result into rewriting a range multiple times, without realizing that this change might already be applied, causing it to swallow or duplicate some bytes. 2. The codemods were mutating the `Candidate` object, however since the `Candidate` parsing is _cached_ in core, it would sometimes return the same instance. This is an issue especially since we monkey patch the prefix to `null` when migrating prefixed candidates. This means that a candidate would be cached that would be _invalid relative to the real design system_. We fixed this by making sure the mutations would only be applied to clones of the `Candidate` and I changed the `DesignSystem` API to return `ReadOnly<T>` versions of these candidates. A better solution would maybe be to disable the cache at all but this requires broader changes in Core.
2025-12-08 21:36:08 +00:00 · 2024-10-08 18:06:43 +02:00 · 2024-10-08 18:06:43 +02:00 · 1467dab59e
commit 1467dab59e
parent fdb90ae630
12 changed files with 142 additions and 24 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -17,7 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 - Don’t crash when scanning a candidate equal to the configured prefix ([#14588](https://github.com/tailwindlabs/tailwindcss/pull/14588))
 - Ensure there's always a space before `!important` when stringifying CSS ([#14611](https://github.com/tailwindlabs/tailwindcss/pull/14611))
- _Experimental_: Ensure CSS before a layer stays unlayered when running codemods ([#14596](https://github.com/tailwindlabs/tailwindcss/pull/14596))
+- _Upgrade (experimental)_: Ensure CSS before a layer stays unlayered when running codemods ([#14596](https://github.com/tailwindlabs/tailwindcss/pull/14596))
+- _Upgrade (experimental)_: Resolve issues where some prefixed candidates were not properly migrated ([#14600](https://github.com/tailwindlabs/tailwindcss/pull/14600))

 ## [4.0.0-alpha.26] - 2024-10-03

--- a/crates/oxide/src/parser.rs
+++ b/crates/oxide/src/parser.rs
@ -861,13 +861,17 @@ impl<'a> Extractor<'a> {
                ParseAction::SingleCandidate(candidate)
            }
            Bracketing::Included(sliceable) | Bracketing::Wrapped(sliceable) => {
-                let parts = vec![candidate, sliceable];
-                let parts = parts
-                    .into_iter()
-                    .filter(|v| !v.is_empty())
-                    .collect::<Vec<_>>();
+                if candidate == sliceable {
+                    ParseAction::SingleCandidate(candidate)
+                } else {
+                    let parts = vec![candidate, sliceable];
+                    let parts = parts
+                        .into_iter()
+                        .filter(|v| !v.is_empty())
+                        .collect::<Vec<_>>();

-                ParseAction::MultipleCandidates(parts)
+                    ParseAction::MultipleCandidates(parts)
+                }
            }
        }
    }
@ -1185,7 +1189,7 @@ mod test {
    fn bad_003() {
        // TODO: This seems… wrong
        let candidates = run(r"[𕤵:]", false);
-        assert_eq!(candidates, vec!["𕤵", "𕤵:"]);
+        assert_eq!(candidates, vec!["𕤵", "𕤵:",]);
    }

    #[test]
@ -1436,4 +1440,15 @@ mod test {
            .unwrap();
        assert_eq!(result, Some("[.foo_&]:px-[0]"));
    }
+
+    #[test]
+    fn does_not_emit_the_same_slice_multiple_times() {
+        let candidates: Vec<_> =
+            Extractor::with_positions("<div class=\"flex\"></div>".as_bytes(), Default::default())
+                .into_iter()
+                .map(|(s, p)| unsafe { (std::str::from_utf8_unchecked(s), p) })
+                .collect();
+
+        assert_eq!(candidates, vec![("div", 1), ("class", 5), ("flex", 12),]);
+    }
 }
--- a/integrations/upgrade/index.test.ts
+++ b/integrations/upgrade/index.test.ts
@ -264,3 +264,91 @@ test(
    )
  },
 )
+
+test(
+  `migrates prefixes even if other files have unprefixed versions of the candidate`,
+  {
+    fs: {
+      'package.json': json`
+        {
+          "dependencies": {
+            "@tailwindcss/upgrade": "workspace:^"
+          }
+        }
+      `,
+      'tailwind.config.js': js`
+        /** @type {import('tailwindcss').Config} */
+        module.exports = {
+          content: ['./src/**/*.{html,js}'],
+          prefix: 'tw__',
+        }
+      `,
+      'src/index.html': html`
+        <div class="flex"></div>
+      `,
+      'src/other.html': html`
+        <div class="tw__flex"></div>
+      `,
+      'src/input.css': css`
+        @tailwind base;
+        @tailwind components;
+        @tailwind utilities;
+      `,
+    },
+  },
+  async ({ exec, fs }) => {
+    await exec('npx @tailwindcss/upgrade -c tailwind.config.js')
+
+    await fs.expectFileToContain('src/index.html', html`
+        <div class="flex"></div>
+      `)
+    await fs.expectFileToContain('src/other.html', html`
+        <div class="tw:flex"></div>
+      `)
+  },
+)
+
+test(
+  `prefixed variants do not cause their unprefixed counterparts to be valid`,
+  {
+    fs: {
+      'package.json': json`
+        {
+          "dependencies": {
+            "@tailwindcss/upgrade": "workspace:^"
+          }
+        }
+      `,
+      'tailwind.config.js': js`
+        /** @type {import('tailwindcss').Config} */
+        module.exports = {
+          content: ['./src/**/*.{html,js}'],
+          prefix: 'tw__',
+        }
+      `,
+      'src/index.html': html`
+        <div class="tw__bg-gradient-to-t"></div>
+      `,
+      'src/other.html': html`
+        <div class="bg-gradient-to-t"></div>
+      `,
+    },
+  },
+  async ({ exec, fs }) => {
+    await exec('npx @tailwindcss/upgrade -c tailwind.config.js')
+
+    await fs.expectFileToContain(
+      'src/index.html',
+      html`
+        <div class="tw:bg-linear-to-t"></div>
+      `,
+    )
+
+    await fs.expectFileToContain(
+      'src/other.html',
+      html`
+        <div class="bg-gradient-to-t"></div>
+      `,
+    )
+  },
+)
--- a/integrations/utils.ts
+++ b/integrations/utils.ts
@ -74,7 +74,7 @@ export function test(
 ) {
  return (only || (!process.env.CI && debug) ? defaultTest.only : defaultTest)(
    name,
-    { timeout: TEST_TIMEOUT, retry: 3 },
+    { timeout: TEST_TIMEOUT, retry: debug ? 0 : 3 },
    async (options) => {
      let rootDir = debug ? path.join(REPO_ROOT, '.debug') : TMP_ROOT
      await fs.mkdir(rootDir, { recursive: true })
--- a/packages/@tailwindcss-upgrade/src/template/candidates.test.ts
+++ b/packages/@tailwindcss-upgrade/src/template/candidates.test.ts
@ -14,7 +14,7 @@ test('extracts candidates with positions from a template', async () => {
    base: __dirname,
  })

-  let candidates = await extractRawCandidates(content)
+  let candidates = await extractRawCandidates(content, 'html')
  let validCandidates = candidates.filter(
    ({ rawCandidate }) => designSystem.parseCandidate(rawCandidate).length > 0,
  )
@ -60,7 +60,7 @@ test('replaces the right positions for a candidate', async () => {
    base: __dirname,
  })

-  let candidates = await extractRawCandidates(content)
+  let candidates = await extractRawCandidates(content, 'html')

  let candidate = candidates.find(
    ({ rawCandidate }) => designSystem.parseCandidate(rawCandidate).length > 0,
--- a/packages/@tailwindcss-upgrade/src/template/codemods/automatic-var-injection.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/automatic-var-injection.ts
@ -1,6 +1,6 @@
 import type { Config } from 'tailwindcss'
 import { walk, WalkAction } from '../../../../tailwindcss/src/ast'
-import type { Candidate, Variant } from '../../../../tailwindcss/src/candidate'
+import { type Candidate, type Variant } from '../../../../tailwindcss/src/candidate'
 import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
 import { printCandidate } from '../candidates'

@ -9,7 +9,11 @@ export function automaticVarInjection(
  _userConfig: Config,
  rawCandidate: string,
 ): string {
-  for (let candidate of designSystem.parseCandidate(rawCandidate)) {
+  for (let readonlyCandidate of designSystem.parseCandidate(rawCandidate)) {
+    // The below logic makes extended use of mutation. Since candidates in the
+    // DesignSystem are cached, we can't mutate them directly.
+    let candidate = structuredClone(readonlyCandidate) as Candidate
+
    let didChange = false

    // Add `var(…)` in modifier position, e.g.:
--- a/packages/@tailwindcss-upgrade/src/template/codemods/bg-gradient.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/bg-gradient.ts
@ -17,8 +17,10 @@ export function bgGradient(
        continue
      }

-      candidate.root = `bg-linear-to-${direction}`
-      return printCandidate(designSystem, candidate)
+      return printCandidate(designSystem, {
+        ...candidate,
+        root: `bg-linear-to-${direction}`,
+      })
    }
  }
  return rawCandidate
--- a/packages/@tailwindcss-upgrade/src/template/codemods/important.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/important.ts
@ -1,4 +1,5 @@
 import type { Config } from 'tailwindcss'
+import { parseCandidate } from '../../../../tailwindcss/src/candidate'
 import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
 import { printCandidate } from '../candidates'

@ -19,7 +20,7 @@ export function important(
  _userConfig: Config,
  rawCandidate: string,
 ): string {
-  for (let candidate of designSystem.parseCandidate(rawCandidate)) {
+  for (let candidate of parseCandidate(rawCandidate, designSystem)) {
    if (candidate.important && candidate.raw[candidate.raw.length - 1] !== '!') {
      // The printCandidate function will already put the exclamation mark in
      // the right place, so we just need to mark this candidate as requiring a
--- a/packages/@tailwindcss-upgrade/src/template/codemods/prefix.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/prefix.ts
@ -1,5 +1,5 @@
 import type { Config } from 'tailwindcss'
-import type { Candidate } from '../../../../tailwindcss/src/candidate'
+import { parseCandidate, type Candidate } from '../../../../tailwindcss/src/candidate'
 import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
 import { segment } from '../../../../tailwindcss/src/utils/segment'
 import { printCandidate } from '../candidates'
@ -24,7 +24,10 @@ export function prefix(
    let unprefixedCandidate =
      rawCandidate.slice(0, v3Base.start) + v3Base.base + rawCandidate.slice(v3Base.end)

-    let candidates = designSystem.parseCandidate(unprefixedCandidate)
+    // Note: This is not a valid candidate in the original DesignSystem, so we
+    // can not use the `DesignSystem#parseCandidate` API here or otherwise this
+    // invalid candidate will be cached.
+    let candidates = [...parseCandidate(unprefixedCandidate, designSystem)]
    if (candidates.length > 0) {
      candidate = candidates[0]
    }
--- a/packages/@tailwindcss-upgrade/src/template/codemods/variant-order.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/variant-order.ts
@ -1,6 +1,6 @@
 import type { Config } from 'tailwindcss'
 import { walk, type AstNode } from '../../../../tailwindcss/src/ast'
-import type { Variant } from '../../../../tailwindcss/src/candidate'
+import { type Variant } from '../../../../tailwindcss/src/candidate'
 import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
 import { printCandidate } from '../candidates'

--- a/packages/@tailwindcss-upgrade/src/template/migrate.ts
+++ b/packages/@tailwindcss-upgrade/src/template/migrate.ts
@ -1,5 +1,5 @@
 import fs from 'node:fs/promises'
-import path from 'node:path'
+import path, { extname } from 'node:path'
 import type { Config } from 'tailwindcss'
 import type { DesignSystem } from '../../../tailwindcss/src/design-system'
 import { extractRawCandidates, replaceCandidateInContent } from './candidates'
@ -38,8 +38,9 @@ export default async function migrateContents(
  designSystem: DesignSystem,
  userConfig: Config,
  contents: string,
+  extension: string,
 ): Promise<string> {
-  let candidates = await extractRawCandidates(contents)
+  let candidates = await extractRawCandidates(contents, extension)

  // Sort candidates by starting position desc
  candidates.sort((a, z) => z.start - a.start)
@ -60,5 +61,8 @@ export async function migrate(designSystem: DesignSystem, userConfig: Config, fi
  let fullPath = path.resolve(process.cwd(), file)
  let contents = await fs.readFile(fullPath, 'utf-8')

-  await fs.writeFile(fullPath, await migrateContents(designSystem, userConfig, contents))
+  await fs.writeFile(
+    fullPath,
+    await migrateContents(designSystem, userConfig, contents, extname(file)),
+  )
 }
--- a/packages/tailwindcss/src/design-system.ts
+++ b/packages/tailwindcss/src/design-system.ts
@ -22,8 +22,8 @@ export type DesignSystem = {
  getClassList(): ClassEntry[]
  getVariants(): VariantEntry[]

-  parseCandidate(candidate: string): Candidate[]
-  parseVariant(variant: string): Variant | null
+  parseCandidate(candidate: string): Readonly<Candidate>[]
+  parseVariant(variant: string): Readonly<Variant> | null
  compileAstNodes(candidate: Candidate): ReturnType<typeof compileAstNodes>

  getVariantOrder(): Map<Variant, number>