Skip to content

Commit 55a330b

Browse files
philipp-spiessRobinMalfait
authored andcommitted
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.
1 parent efe9ac4 commit 55a330b

File tree

12 files changed

+142
-24
lines changed

12 files changed

+142
-24
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717

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

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

crates/oxide/src/parser.rs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -861,13 +861,17 @@ impl<'a> Extractor<'a> {
861861
ParseAction::SingleCandidate(candidate)
862862
}
863863
Bracketing::Included(sliceable) | Bracketing::Wrapped(sliceable) => {
864-
let parts = vec![candidate, sliceable];
865-
let parts = parts
866-
.into_iter()
867-
.filter(|v| !v.is_empty())
868-
.collect::<Vec<_>>();
864+
if candidate == sliceable {
865+
ParseAction::SingleCandidate(candidate)
866+
} else {
867+
let parts = vec![candidate, sliceable];
868+
let parts = parts
869+
.into_iter()
870+
.filter(|v| !v.is_empty())
871+
.collect::<Vec<_>>();
869872

870-
ParseAction::MultipleCandidates(parts)
873+
ParseAction::MultipleCandidates(parts)
874+
}
871875
}
872876
}
873877
}
@@ -1185,7 +1189,7 @@ mod test {
11851189
fn bad_003() {
11861190
// TODO: This seems… wrong
11871191
let candidates = run(r"[𕤵:]", false);
1188-
assert_eq!(candidates, vec!["𕤵", "𕤵:"]);
1192+
assert_eq!(candidates, vec!["𕤵", "𕤵:",]);
11891193
}
11901194

11911195
#[test]
@@ -1436,4 +1440,15 @@ mod test {
14361440
.unwrap();
14371441
assert_eq!(result, Some("[.foo_&]:px-[0]"));
14381442
}
1443+
1444+
#[test]
1445+
fn does_not_emit_the_same_slice_multiple_times() {
1446+
let candidates: Vec<_> =
1447+
Extractor::with_positions("<div class=\"flex\"></div>".as_bytes(), Default::default())
1448+
.into_iter()
1449+
.map(|(s, p)| unsafe { (std::str::from_utf8_unchecked(s), p) })
1450+
.collect();
1451+
1452+
assert_eq!(candidates, vec![("div", 1), ("class", 5), ("flex", 12),]);
1453+
}
14391454
}

integrations/upgrade/index.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,3 +555,91 @@ test(
555555
`)
556556
},
557557
)
558+
559+
test(
560+
`migrates prefixes even if other files have unprefixed versions of the candidate`,
561+
{
562+
fs: {
563+
'package.json': json`
564+
{
565+
"dependencies": {
566+
"@tailwindcss/upgrade": "workspace:^"
567+
}
568+
}
569+
`,
570+
'tailwind.config.js': js`
571+
/** @type {import('tailwindcss').Config} */
572+
module.exports = {
573+
content: ['./src/**/*.{html,js}'],
574+
prefix: 'tw__',
575+
}
576+
`,
577+
'src/index.html': html`
578+
<div class="flex"></div>
579+
`,
580+
'src/other.html': html`
581+
<div class="tw__flex"></div>
582+
`,
583+
'src/input.css': css`
584+
@tailwind base;
585+
@tailwind components;
586+
@tailwind utilities;
587+
`,
588+
},
589+
},
590+
async ({ exec, fs }) => {
591+
await exec('npx @tailwindcss/upgrade -c tailwind.config.js')
592+
593+
await fs.expectFileToContain('src/index.html', html`
594+
<div class="flex"></div>
595+
`)
596+
await fs.expectFileToContain('src/other.html', html`
597+
<div class="tw:flex"></div>
598+
`)
599+
},
600+
)
601+
602+
test(
603+
`prefixed variants do not cause their unprefixed counterparts to be valid`,
604+
{
605+
fs: {
606+
'package.json': json`
607+
{
608+
"dependencies": {
609+
"@tailwindcss/upgrade": "workspace:^"
610+
}
611+
}
612+
`,
613+
'tailwind.config.js': js`
614+
/** @type {import('tailwindcss').Config} */
615+
module.exports = {
616+
content: ['./src/**/*.{html,js}'],
617+
prefix: 'tw__',
618+
}
619+
`,
620+
'src/index.html': html`
621+
<div class="tw__bg-gradient-to-t"></div>
622+
`,
623+
'src/other.html': html`
624+
<div class="bg-gradient-to-t"></div>
625+
`,
626+
},
627+
},
628+
async ({ exec, fs }) => {
629+
await exec('npx @tailwindcss/upgrade -c tailwind.config.js')
630+
631+
await fs.expectFileToContain(
632+
'src/index.html',
633+
html`
634+
<div class="tw:bg-linear-to-t"></div>
635+
`,
636+
)
637+
638+
await fs.expectFileToContain(
639+
'src/other.html',
640+
html`
641+
<div class="bg-gradient-to-t"></div>
642+
`,
643+
)
644+
},
645+
)

integrations/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export function test(
7575
) {
7676
return (only || (!process.env.CI && debug) ? defaultTest.only : defaultTest)(
7777
name,
78-
{ timeout: TEST_TIMEOUT, retry: 3 },
78+
{ timeout: TEST_TIMEOUT, retry: debug ? 0 : 3 },
7979
async (options) => {
8080
let rootDir = debug ? path.join(REPO_ROOT, '.debug') : TMP_ROOT
8181
await fs.mkdir(rootDir, { recursive: true })

packages/@tailwindcss-upgrade/src/template/candidates.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ test('extracts candidates with positions from a template', async () => {
1414
base: __dirname,
1515
})
1616

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

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

6565
let candidate = candidates.find(
6666
({ rawCandidate }) => designSystem.parseCandidate(rawCandidate).length > 0,

packages/@tailwindcss-upgrade/src/template/codemods/automatic-var-injection.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Config } from 'tailwindcss'
22
import { walk, WalkAction } from '../../../../tailwindcss/src/ast'
3-
import type { Candidate, Variant } from '../../../../tailwindcss/src/candidate'
3+
import { type Candidate, type Variant } from '../../../../tailwindcss/src/candidate'
44
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
55
import { printCandidate } from '../candidates'
66

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

1519
// Add `var(…)` in modifier position, e.g.:

packages/@tailwindcss-upgrade/src/template/codemods/bg-gradient.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ export function bgGradient(
1717
continue
1818
}
1919

20-
candidate.root = `bg-linear-to-${direction}`
21-
return printCandidate(designSystem, candidate)
20+
return printCandidate(designSystem, {
21+
...candidate,
22+
root: `bg-linear-to-${direction}`,
23+
})
2224
}
2325
}
2426
return rawCandidate

packages/@tailwindcss-upgrade/src/template/codemods/important.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from 'tailwindcss'
2+
import { parseCandidate } from '../../../../tailwindcss/src/candidate'
23
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
34
import { printCandidate } from '../candidates'
45

@@ -19,7 +20,7 @@ export function important(
1920
_userConfig: Config,
2021
rawCandidate: string,
2122
): string {
22-
for (let candidate of designSystem.parseCandidate(rawCandidate)) {
23+
for (let candidate of parseCandidate(rawCandidate, designSystem)) {
2324
if (candidate.important && candidate.raw[candidate.raw.length - 1] !== '!') {
2425
// The printCandidate function will already put the exclamation mark in
2526
// the right place, so we just need to mark this candidate as requiring a

packages/@tailwindcss-upgrade/src/template/codemods/prefix.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Config } from 'tailwindcss'
2-
import type { Candidate } from '../../../../tailwindcss/src/candidate'
2+
import { parseCandidate, type Candidate } from '../../../../tailwindcss/src/candidate'
33
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
44
import { segment } from '../../../../tailwindcss/src/utils/segment'
55
import { printCandidate } from '../candidates'
@@ -24,7 +24,10 @@ export function prefix(
2424
let unprefixedCandidate =
2525
rawCandidate.slice(0, v3Base.start) + v3Base.base + rawCandidate.slice(v3Base.end)
2626

27-
let candidates = designSystem.parseCandidate(unprefixedCandidate)
27+
// Note: This is not a valid candidate in the original DesignSystem, so we
28+
// can not use the `DesignSystem#parseCandidate` API here or otherwise this
29+
// invalid candidate will be cached.
30+
let candidates = [...parseCandidate(unprefixedCandidate, designSystem)]
2831
if (candidates.length > 0) {
2932
candidate = candidates[0]
3033
}

packages/@tailwindcss-upgrade/src/template/codemods/variant-order.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Config } from 'tailwindcss'
22
import { walk, type AstNode } from '../../../../tailwindcss/src/ast'
3-
import type { Variant } from '../../../../tailwindcss/src/candidate'
3+
import { type Variant } from '../../../../tailwindcss/src/candidate'
44
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
55
import { printCandidate } from '../candidates'
66

0 commit comments

Comments
 (0)