Skip to content

Commit 1ada8e0

Browse files
authored
Make candidate template migrations faster (#18025)
This PR makes the migrations for templates much faster. To make this work, I also had to move things around a bit (so you might want to check this PR commit by commit). I also solved an issue by restructuring the code. ### Performance For starters, we barely applied any caching when migrating candidates from α to β. The problem with this is that in big projects the same candidates will appear _everywhere_, so caching is going to be useful here. One of the reasons why we didn't do any caching is that some migrations were checking if a migration is actually safe to do. To do this, we were checking the `location` (the location of the candidate in the template). Since this location is unique for each template, caching was not possible. So the first order of business was to hoist the `isSafeMigration` check up as the very first thing we do in the migration. If we do this first, then the only remaining code relies on the `DesignSystem`, `UserConfig` and `rawCandidate`. In a project, the `DesignSystem` and `UserConfig` will be the same during the migration, only the `rawCandidate` will be different which means that we can move all this logic in a good old `DefaultMap` and cache the heck out of it. Running the numbers on our Tailwind Plus repo, this results in: ``` Total seen candidates: 2 211 844 Total migrated candidates: 7 775 Cache hits: 1 575 700 ``` That's a lot of work we _don't_ have to do. Looking at the timings, the template migration step goes from ~45s to ~10s because of this. Another big benefit of this is that this makes migrations _actually_ safe. Before we were checking if a migration was safe to do in specific migrations. But other migrations were still printing the candidate which could still result in an unsafe migration. For example when migrating the `blur` and the `shadow` classes, the `isSafeMigration` was used. But if the input was `!flex` then the safety check wasn't even checked in this specific migration. ### Safe migrations Also made some changes to the `isSafeMigration` logic itself. We used to start by checking the location, but thinking about the problem again, the actual big problem we were running into is classes that are short like `blur`, and `shadow` because they could be used in other contexts than a Tailwind CSS class. Inverting this logic means that more specific Tailwind CSS classes will very likely _not_ cause any issues at all. For example: - If you have variants: `hover:focus:flex` - If you have arbitrary properties: `[color:red]` - If you have arbitrary values: `bg-[red]` - If you have a modifier: `bg-red-500/50` - If you have a `-` in the name: `bg-red-500` Even better if we can't parse a candidate at all, we can skip the migrations all together. This brings us to the issue in #17974, one of the issues was already solved by just hoisting the `isSafeMigration`. But to make the issue was completely solved I also made sure that in Vue attributes like `:active="…"` are also considered unsafe (note: `:class` is allowed). Last but not least, in case of the `!duration` that got replaced with `duration!` was solved by verifying that the candidate actually produces valid CSS. We can compute the signature for this class. The reason this wasn't thrown away earlier is because we can correctly parse `duration` but `duration` on its own doesn't exist, `duration-<number>` does exist as a functional utility which is why it parsed in the first place. Fixes: #17974 ## Test plan 1. Ran the tool on our Tailwind UI Templates repo to compare the new output with the "old" behavior and there were no differences in output. 2. Ran the tool on our Tailwind Plus repo, and the template migration step went from ~45s to ~10s. 3. Added additional tests to verify the issues in #17974 are fixed. [ci-all] let's run this on all CI platforms...
1 parent 6fb98d2 commit 1ada8e0

14 files changed

+243
-235
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
- Ensure that running the Standalone build does not leave temporary files behind ([#17981](https://github.com/tailwindlabs/tailwindcss/pull/17981))
1818
- Fix `-rotate-*` utilities with arbitrary values ([#18014](https://github.com/tailwindlabs/tailwindcss/pull/18014))
1919
- Upgrade: Change casing of utilities with named values to kebab-case to match updated theme variables ([#18017](https://github.com/tailwindlabs/tailwindcss/pull/18017))
20+
- Upgrade: Fix unsafe migrations in Vue files ([#18025](https://github.com/tailwindlabs/tailwindcss/pull/18025))
2021
- Ignore custom variants using `:merge(…)` selectors in legacy JS plugins ([#18020](https://github.com/tailwindlabs/tailwindcss/pull/18020))
2122

2223
### Added
2324

2425
- Upgrade: Migrate bare values to named values ([#18000](https://github.com/tailwindlabs/tailwindcss/pull/18000))
26+
- Upgrade: Make candidate template migrations faster using caching ([#18025](https://github.com/tailwindlabs/tailwindcss/pull/18025))
2527

2628
## [4.1.6] - 2025-05-09
2729

integrations/upgrade/js-config.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ test(
151151
<div
152152
class="[letter-spacing:theme(letterSpacing.superWide)] [line-height:theme(lineHeight.superLoose)]"
153153
></div>
154-
<div class="text-red-superRed/superOpaque leading-superLoose"></div>
154+
<div class="text-superRed/superOpaque leading-superLoose"></div>
155155
`,
156156
'node_modules/my-external-lib/src/template.html': html`
157157
<div class="text-red-500">
@@ -169,7 +169,7 @@ test(
169169
<div
170170
class="[letter-spacing:var(--tracking-super-wide)] [line-height:var(--leading-super-loose)]"
171171
></div>
172-
<div class="text-red-super-red/super-opaque leading-super-loose"></div>
172+
<div class="text-super-red/super-opaque leading-super-loose"></div>
173173
174174
--- src/input.css ---
175175
@import 'tailwindcss';
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { __unstable__loadDesignSystem } from '@tailwindcss/node'
2+
import { expect, test, vi } from 'vitest'
3+
import * as versions from '../../utils/version'
4+
import { migrateCandidate } from './migrate'
5+
vi.spyOn(versions, 'isMajor').mockReturnValue(true)
6+
7+
test('does not replace classes in invalid positions', async () => {
8+
let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
9+
base: __dirname,
10+
})
11+
12+
async function shouldNotReplace(example: string, candidate = '!border') {
13+
expect(
14+
await migrateCandidate(designSystem, {}, candidate, {
15+
contents: example,
16+
start: example.indexOf(candidate),
17+
end: example.indexOf(candidate) + candidate.length,
18+
}),
19+
).toEqual(candidate)
20+
}
21+
22+
await shouldNotReplace(`let notBorder = !border \n`)
23+
await shouldNotReplace(`{ "foo": !border.something + ""}\n`)
24+
await shouldNotReplace(`<div v-if="something && !border"></div>\n`)
25+
await shouldNotReplace(`<div v-else-if="something && !border"></div>\n`)
26+
await shouldNotReplace(`<div v-show="something && !border"></div>\n`)
27+
await shouldNotReplace(`<div v-if="!border || !border"></div>\n`)
28+
await shouldNotReplace(`<div v-else-if="!border || !border"></div>\n`)
29+
await shouldNotReplace(`<div v-show="!border || !border"></div>\n`)
30+
await shouldNotReplace(`<div v-if="!border"></div>\n`)
31+
await shouldNotReplace(`<div v-else-if="!border"></div>\n`)
32+
await shouldNotReplace(`<div v-show="!border"></div>\n`)
33+
await shouldNotReplace(`<div x-if="!border"></div>\n`)
34+
35+
await shouldNotReplace(`let notShadow = shadow \n`, 'shadow')
36+
await shouldNotReplace(`{ "foo": shadow.something + ""}\n`, 'shadow')
37+
await shouldNotReplace(`<div v-if="something && shadow"></div>\n`, 'shadow')
38+
await shouldNotReplace(`<div v-else-if="something && shadow"></div>\n`, 'shadow')
39+
await shouldNotReplace(`<div v-show="something && shadow"></div>\n`, 'shadow')
40+
await shouldNotReplace(`<div v-if="shadow || shadow"></div>\n`, 'shadow')
41+
await shouldNotReplace(`<div v-else-if="shadow || shadow"></div>\n`, 'shadow')
42+
await shouldNotReplace(`<div v-show="shadow || shadow"></div>\n`, 'shadow')
43+
await shouldNotReplace(`<div v-if="shadow"></div>\n`, 'shadow')
44+
await shouldNotReplace(`<div v-else-if="shadow"></div>\n`, 'shadow')
45+
await shouldNotReplace(`<div v-show="shadow"></div>\n`, 'shadow')
46+
await shouldNotReplace(`<div x-if="shadow"></div>\n`, 'shadow')
47+
await shouldNotReplace(
48+
`<div style={{filter: 'drop-shadow(30px 10px 4px #4444dd)'}}/>\n`,
49+
'shadow',
50+
)
51+
52+
// Next.js Image placeholder cases
53+
await shouldNotReplace(`<Image placeholder="blur" src="/image.jpg" />`, 'blur')
54+
await shouldNotReplace(`<Image placeholder={'blur'} src="/image.jpg" />`, 'blur')
55+
await shouldNotReplace(`<Image placeholder={blur} src="/image.jpg" />`, 'blur')
56+
57+
// https://github.com/tailwindlabs/tailwindcss/issues/17974
58+
await shouldNotReplace('<div v-if="!duration">', '!duration')
59+
await shouldNotReplace('<div :active="!duration">', '!duration')
60+
await shouldNotReplace('<div :active="!visible">', '!visible')
61+
})

packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,84 @@
1+
import { parseCandidate } from '../../../../tailwindcss/src/candidate'
2+
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
3+
import * as version from '../../utils/version'
4+
15
const QUOTES = ['"', "'", '`']
26
const LOGICAL_OPERATORS = ['&&', '||', '?', '===', '==', '!=', '!==', '>', '>=', '<', '<=']
37
const CONDITIONAL_TEMPLATE_SYNTAX = [
48
// Vue
59
/v-else-if=['"]$/,
610
/v-if=['"]$/,
711
/v-show=['"]$/,
12+
/(?<!:?class)=['"]$/,
813

914
// Alpine
1015
/x-if=['"]$/,
1116
/x-show=['"]$/,
1217
]
1318
const NEXT_PLACEHOLDER_PROP = /placeholder=\{?['"]$/
1419

15-
export function isSafeMigration(location: { contents: string; start: number; end: number }) {
20+
export function isSafeMigration(
21+
rawCandidate: string,
22+
location: { contents: string; start: number; end: number },
23+
designSystem: DesignSystem,
24+
): boolean {
25+
let [candidate] = Array.from(parseCandidate(rawCandidate, designSystem))
26+
27+
// If we can't parse the candidate, then it's not a candidate at all. However,
28+
// we could be dealing with legacy classes like `tw__flex` in Tailwind CSS v3
29+
// land, which also wouldn't parse.
30+
//
31+
// So let's only skip if we couldn't parse and we are not in Tailwind CSS v3.
32+
//
33+
if (!candidate && version.isGreaterThan(3)) {
34+
return true
35+
}
36+
37+
// Parsed a candidate succesfully, verify if it's a valid candidate
38+
else if (candidate) {
39+
// When we have variants, we can assume that the candidate is safe to migrate
40+
// because that requires colons.
41+
//
42+
// E.g.: `hover:focus:flex`
43+
if (candidate.variants.length > 0) {
44+
return true
45+
}
46+
47+
// When we have an arbitrary property, the candidate has such a particular
48+
// structure it's very likely to be safe.
49+
//
50+
// E.g.: `[color:red]`
51+
if (candidate.kind === 'arbitrary') {
52+
return true
53+
}
54+
55+
// A static candidate is very likely safe if it contains a dash.
56+
//
57+
// E.g.: `items-center`
58+
if (candidate.kind === 'static' && candidate.root.includes('-')) {
59+
return true
60+
}
61+
62+
// A functional candidate is very likely safe if it contains a value (which
63+
// implies a `-`). Or if the root contains a dash.
64+
//
65+
// E.g.: `bg-red-500`, `bg-position-20`
66+
if (
67+
(candidate.kind === 'functional' && candidate.value !== null) ||
68+
(candidate.kind === 'functional' && candidate.root.includes('-'))
69+
) {
70+
return true
71+
}
72+
73+
// If the candidate contains a modifier, it's very likely to be safe because
74+
// it implies that it contains a `/`.
75+
//
76+
// E.g.: `text-sm/7`
77+
if (candidate.kind === 'functional' && candidate.modifier) {
78+
return true
79+
}
80+
}
81+
1682
let currentLineBeforeCandidate = ''
1783
for (let i = location.start - 1; i >= 0; i--) {
1884
let char = location.contents.at(i)!

packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-utilities.ts

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,27 +51,6 @@ export function migrateArbitraryUtilities(
5151
continue
5252
}
5353

54-
// 1. Canonicalize the value. This might be a bit wasteful because it might
55-
// have been done by other migrations before, but essentially we want to
56-
// canonicalize the arbitrary value to its simplest canonical form. We
57-
// won't be constant folding `calc(…)` expressions (yet?), but we can
58-
// remove unnecessary whitespace (which the `printCandidate` already
59-
// handles for us).
60-
//
61-
// E.g.:
62-
//
63-
// ```
64-
// [display:_flex_] => [display:flex]
65-
// [display:_flex] => [display:flex]
66-
// [display:flex_] => [display:flex]
67-
// [display:flex] => [display:flex]
68-
// ```
69-
//
70-
let canonicalizedCandidate = designSystem.printCandidate(readonlyCandidate)
71-
if (canonicalizedCandidate !== rawCandidate) {
72-
return migrateArbitraryUtilities(designSystem, _userConfig, canonicalizedCandidate)
73-
}
74-
7554
// The below logic makes use of mutation. Since candidates in the
7655
// DesignSystem are cached, we can't mutate them directly.
7756
let candidate = structuredClone(readonlyCandidate) as Writable<typeof readonlyCandidate>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { __unstable__loadDesignSystem } from '@tailwindcss/node'
2+
import { expect, test, vi } from 'vitest'
3+
import * as versions from '../../utils/version'
4+
import { migrateCanonicalizeCandidate } from './migrate-canonicalize-candidate'
5+
vi.spyOn(versions, 'isMajor').mockReturnValue(true)
6+
7+
test.each([
8+
// Normalize whitespace in arbitrary properties
9+
['[display:flex]', '[display:flex]'],
10+
['[display:_flex]', '[display:flex]'],
11+
['[display:flex_]', '[display:flex]'],
12+
['[display:_flex_]', '[display:flex]'],
13+
14+
// Normalize whitespace in `calc` expressions
15+
['w-[calc(100%-2rem)]', 'w-[calc(100%-2rem)]'],
16+
['w-[calc(100%_-_2rem)]', 'w-[calc(100%-2rem)]'],
17+
18+
// Normalize the important modifier
19+
['!flex', 'flex!'],
20+
['flex!', 'flex!'],
21+
])('%s => %s', async (candidate, result) => {
22+
let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
23+
base: __dirname,
24+
})
25+
26+
expect(migrateCanonicalizeCandidate(designSystem, {}, candidate)).toEqual(result)
27+
})
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import type { Config } from '../../../../tailwindcss/src/compat/plugin-api'
2+
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
3+
4+
// Canonicalize the value to its minimal form. This will normalize whitespace,
5+
// and print the important modifier `!` in the correct place.
6+
//
7+
// E.g.:
8+
//
9+
// ```
10+
// [display:_flex_] => [display:flex]
11+
// [display:_flex] => [display:flex]
12+
// [display:flex_] => [display:flex]
13+
// [display:flex] => [display:flex]
14+
// ```
15+
//
16+
export function migrateCanonicalizeCandidate(
17+
designSystem: DesignSystem,
18+
_userConfig: Config | null,
19+
rawCandidate: string,
20+
) {
21+
for (let readonlyCandidate of designSystem.parseCandidate(rawCandidate)) {
22+
let canonicalizedCandidate = designSystem.printCandidate(readonlyCandidate)
23+
if (canonicalizedCandidate !== rawCandidate) {
24+
return canonicalizedCandidate
25+
}
26+
}
27+
28+
return rawCandidate
29+
}

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

Lines changed: 0 additions & 68 deletions
This file was deleted.

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

Lines changed: 0 additions & 48 deletions
This file was deleted.

0 commit comments

Comments
 (0)