Skip to content

Commit 30be24b

Browse files
authored
Fix false-positive migrations in addEventListener and JavaScript variable names (#18718)
This PR fixes 2 false-positives when running the upgrade tool on a Tailwind CSS v3 project converting it to a Tailwind CSS v4 project. The issue occurs around migrations with short simple names that have a meaning outside if Tailwind CSS, e.g. `blur` and `outline`. This PR fixes 2 such cases: 1. The `addEventListener` case: ```js document.addEventListener('blur', handleBlur) ``` We do this by special casing the `addEventListener(` case and making sure the first argument to `addEventListener` is never migrated. 2. A JavaScript variable with default value: ```js function foo({ foo = "bar", outline = true, baz = "qux" }) { // ... } ``` The bug is relatively subtle here, but it has actually nothing to do with `outline` itself, but rather the fact that some quote character came before and after it on the same line... One of our heuristics for determining if a migration on these small words is safe, is to ensure that the candidate is inside of a string. Since we didn't do any kind of quote balancing, we would consider the `outline` to be inside of a string, even though it is not. So to actually solve this, we do some form of quote balancing to ensure that it's _not_ inside of a string in this case. Additionally, this PR also introduces a small refactor to the `is-safe-migration.test.ts` file where we now use a `test.each` to ensure that failing tests in the middle don't prevent the rest of the tests from running. ### Test plan 1. Added dedicated tests for the cases mentioned in the issue (#18675). 2. Added a few more tests with various forms of whitespace. Fixes: #18675
1 parent 3468bcf commit 30be24b

File tree

3 files changed

+129
-56
lines changed

3 files changed

+129
-56
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2424
- Improve error messages when encountering invalid functional utility names ([#18568](https://github.com/tailwindlabs/tailwindcss/pull/18568))
2525
- Don’t output CSS objects with false or undefined in the AST ([#18571](https://github.com/tailwindlabs/tailwindcss/pull/18571))
2626
- Add option to disable url rewriting in `@tailwindcss/postcss` ([#18321](https://github.com/tailwindlabs/tailwindcss/pull/18321))
27+
- Fix false-positive migrations in `addEventListener` and JavaScript variable names ([#18718](https://github.com/tailwindlabs/tailwindcss/pull/18718))
2728

2829
## [4.1.11] - 2025-06-26
2930

Lines changed: 86 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,103 @@
11
import { __unstable__loadDesignSystem } from '@tailwindcss/node'
2-
import { expect, test, vi } from 'vitest'
2+
import { describe, expect, test, vi } from 'vitest'
33
import * as versions from '../../utils/version'
44
import { migrateCandidate } from './migrate'
55
vi.spyOn(versions, 'isMajor').mockReturnValue(true)
66

7-
test('does not replace classes in invalid positions', async () => {
7+
describe('is-safe-migration', async () => {
88
let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
99
base: __dirname,
1010
})
1111

12-
async function shouldNotReplace(example: string, candidate = '!border') {
12+
test.each([
13+
[`let notBorder = !border \n`, '!border'],
14+
[`{ "foo": !border.something + ""}\n`, '!border'],
15+
[`<div v-if="something && !border"></div>\n`, '!border'],
16+
[`<div v-else-if="something && !border"></div>\n`, '!border'],
17+
[`<div v-show="something && !border"></div>\n`, '!border'],
18+
[`<div v-if="!border || !border"></div>\n`, '!border'],
19+
[`<div v-else-if="!border || !border"></div>\n`, '!border'],
20+
[`<div v-show="!border || !border"></div>\n`, '!border'],
21+
[`<div v-if="!border"></div>\n`, '!border'],
22+
[`<div v-else-if="!border"></div>\n`, '!border'],
23+
[`<div v-show="!border"></div>\n`, '!border'],
24+
[`<div x-if="!border"></div>\n`, '!border'],
25+
26+
[`let notShadow = shadow \n`, 'shadow'],
27+
[`{ "foo": shadow.something + ""}\n`, 'shadow'],
28+
[`<div v-if="something && shadow"></div>\n`, 'shadow'],
29+
[`<div v-else-if="something && shadow"></div>\n`, 'shadow'],
30+
[`<div v-show="something && shadow"></div>\n`, 'shadow'],
31+
[`<div v-if="shadow || shadow"></div>\n`, 'shadow'],
32+
[`<div v-else-if="shadow || shadow"></div>\n`, 'shadow'],
33+
[`<div v-show="shadow || shadow"></div>\n`, 'shadow'],
34+
[`<div v-if="shadow"></div>\n`, 'shadow'],
35+
[`<div v-else-if="shadow"></div>\n`, 'shadow'],
36+
[`<div v-show="shadow"></div>\n`, 'shadow'],
37+
[`<div x-if="shadow"></div>\n`, 'shadow'],
38+
[`<div style={{filter: 'drop-shadow(30px 10px 4px #4444dd)'}}/>\n`, 'shadow'],
39+
40+
// Next.js Image placeholder cases
41+
[`<Image placeholder="blur" src="/image.jpg" />`, 'blur'],
42+
[`<Image placeholder={'blur'} src="/image.jpg" />`, 'blur'],
43+
[`<Image placeholder={blur} src="/image.jpg" />`, 'blur'],
44+
45+
// https://github.com/tailwindlabs/tailwindcss/issues/17974
46+
['<div v-if="!duration">', '!duration'],
47+
['<div :active="!duration">', '!duration'],
48+
['<div :active="!visible">', '!visible'],
49+
50+
// Alpine/Livewire wire:…
51+
['<x-input.number required="foo" wire:model.blur="coins" />', 'blur'],
52+
53+
// Vue 3 events
54+
[`emit('blur', props.modelValue)\n`, 'blur'],
55+
[`$emit('blur', props.modelValue)\n`, 'blur'],
56+
57+
// JavaScript / TypeScript
58+
[`document.addEventListener('blur',handleBlur)`, 'blur'],
59+
[`document.addEventListener('blur', handleBlur)`, 'blur'],
60+
61+
[`function foo({ outline = true })`, 'outline'],
62+
[`function foo({ before = false, outline = true })`, 'outline'],
63+
[`function foo({before=false,outline=true })`, 'outline'],
64+
[`function foo({outline=true })`, 'outline'],
65+
// https://github.com/tailwindlabs/tailwindcss/issues/18675
66+
[
67+
// With default value
68+
`function foo({ size = "1.25rem", digit, outline = true, textClass = "", className = "" })`,
69+
'outline',
70+
],
71+
[
72+
// Without default value
73+
`function foo({ size = "1.25rem", digit, outline, textClass = "", className = "" })`,
74+
'outline',
75+
],
76+
[
77+
// As the last argument
78+
`function foo({ size = "1.25rem", digit, outline })`,
79+
'outline',
80+
],
81+
[
82+
// As the last argument, but there is techinically another `"` on the same line
83+
`function foo({ size = "1.25rem", digit, outline }): { return "foo" }`,
84+
'outline',
85+
],
86+
[
87+
// Tricky quote balancing
88+
`function foo({ before = "'", outline, after = "'" }): { return "foo" }`,
89+
'outline',
90+
],
91+
92+
[`function foo(blur, foo)`, 'blur'],
93+
[`function foo(blur,foo)`, 'blur'],
94+
])('does not replace classes in invalid positions #%#', async (example, candidate) => {
1395
expect(
1496
await migrateCandidate(designSystem, {}, candidate, {
1597
contents: example,
1698
start: example.indexOf(candidate),
1799
end: example.indexOf(candidate) + candidate.length,
18100
}),
19101
).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-
62-
// Alpine/Livewire wire:…
63-
await shouldNotReplace('<x-input.number required="foo" wire:model.blur="coins" />', 'blur')
64-
65-
// Vue 3 events
66-
await shouldNotReplace(`emit('blur', props.modelValue)\n`, 'blur')
67-
await shouldNotReplace(`$emit('blur', props.modelValue)\n`, 'blur')
102+
})
68103
})

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

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
33
import { DefaultMap } from '../../../../tailwindcss/src/utils/default-map'
44
import * as version from '../../utils/version'
55

6-
const QUOTES = ['"', "'", '`']
76
const LOGICAL_OPERATORS = ['&&', '||', '?', '===', '==', '!=', '!==', '>', '>=', '<', '<=']
87
const CONDITIONAL_TEMPLATE_SYNTAX = [
98
// Vue
@@ -12,13 +11,16 @@ const CONDITIONAL_TEMPLATE_SYNTAX = [
1211
/v-show=['"]$/,
1312
/(?<!:?class)=['"]$/,
1413

14+
// JavaScript / TypeScript
15+
/addEventListener\(['"`]$/,
16+
1517
// Alpine
1618
/x-if=['"]$/,
1719
/x-show=['"]$/,
1820
/wire:[^\s]*?$/,
1921
]
20-
const NEXT_PLACEHOLDER_PROP = /placeholder=\{?['"]$/
21-
const VUE_3_EMIT = /\b\$?emit\(['"]$/
22+
const NEXT_PLACEHOLDER_PROP = /placeholder=\{?['"`]$/
23+
const VUE_3_EMIT = /\b\$?emit\(['"`]$/
2224

2325
export function isSafeMigration(
2426
rawCandidate: string,
@@ -138,8 +140,8 @@ export function isSafeMigration(
138140
}
139141

140142
// Heuristic: Require the candidate to be inside quotes
141-
let isQuoteBeforeCandidate = QUOTES.some((quote) => currentLineBeforeCandidate.includes(quote))
142-
let isQuoteAfterCandidate = QUOTES.some((quote) => currentLineAfterCandidate.includes(quote))
143+
let isQuoteBeforeCandidate = isMiddleOfString(currentLineBeforeCandidate)
144+
let isQuoteAfterCandidate = isMiddleOfString(currentLineAfterCandidate)
143145
if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
144146
return false
145147
}
@@ -210,3 +212,38 @@ const styleBlockRanges = new DefaultMap((source: string) => {
210212
ranges.push(startTag, endTag)
211213
}
212214
})
215+
216+
const BACKSLASH = 0x5c
217+
const DOUBLE_QUOTE = 0x22
218+
const SINGLE_QUOTE = 0x27
219+
const BACKTICK = 0x60
220+
221+
function isMiddleOfString(line: string): boolean {
222+
let currentQuote: number | null = null
223+
224+
for (let i = 0; i < line.length; i++) {
225+
let char = line.charCodeAt(i)
226+
switch (char) {
227+
// Escaped character, skip the next character
228+
case BACKSLASH:
229+
i++
230+
break
231+
232+
case SINGLE_QUOTE:
233+
case DOUBLE_QUOTE:
234+
case BACKTICK:
235+
// Found matching quote, we are outside of a string
236+
if (currentQuote === char) {
237+
currentQuote = null
238+
}
239+
240+
// Found a quote, we are inside a string
241+
else if (currentQuote === null) {
242+
currentQuote = char
243+
}
244+
break
245+
}
246+
}
247+
248+
return currentQuote !== null
249+
}

0 commit comments

Comments
 (0)