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
This commit is contained in:
Robin Malfait
GitHub
parent
3468bcface
commit
30be24b29f
3 changed files with 129 additions and 56 deletions
|
|
@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
- Improve error messages when encountering invalid functional utility names ([#18568](https://github.com/tailwindlabs/tailwindcss/pull/18568))
|
||||
- Don’t output CSS objects with false or undefined in the AST ([#18571](https://github.com/tailwindlabs/tailwindcss/pull/18571))
|
||||
- Add option to disable url rewriting in `@tailwindcss/postcss` ([#18321](https://github.com/tailwindlabs/tailwindcss/pull/18321))
|
||||
- Fix false-positive migrations in `addEventListener` and JavaScript variable names ([#18718](https://github.com/tailwindlabs/tailwindcss/pull/18718))
|
||||
|
||||
## [4.1.11] - 2025-06-26
|
||||
|
||||
|
|
|
|||
|
|
@ -1,15 +1,97 @@
|
|||
import { __unstable__loadDesignSystem } from '@tailwindcss/node'
|
||||
import { expect, test, vi } from 'vitest'
|
||||
import { describe, expect, test, vi } from 'vitest'
|
||||
import * as versions from '../../utils/version'
|
||||
import { migrateCandidate } from './migrate'
|
||||
vi.spyOn(versions, 'isMajor').mockReturnValue(true)
|
||||
|
||||
test('does not replace classes in invalid positions', async () => {
|
||||
describe('is-safe-migration', async () => {
|
||||
let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
|
||||
base: __dirname,
|
||||
})
|
||||
|
||||
async function shouldNotReplace(example: string, candidate = '!border') {
|
||||
test.each([
|
||||
[`let notBorder = !border \n`, '!border'],
|
||||
[`{ "foo": !border.something + ""}\n`, '!border'],
|
||||
[`<div v-if="something && !border"></div>\n`, '!border'],
|
||||
[`<div v-else-if="something && !border"></div>\n`, '!border'],
|
||||
[`<div v-show="something && !border"></div>\n`, '!border'],
|
||||
[`<div v-if="!border || !border"></div>\n`, '!border'],
|
||||
[`<div v-else-if="!border || !border"></div>\n`, '!border'],
|
||||
[`<div v-show="!border || !border"></div>\n`, '!border'],
|
||||
[`<div v-if="!border"></div>\n`, '!border'],
|
||||
[`<div v-else-if="!border"></div>\n`, '!border'],
|
||||
[`<div v-show="!border"></div>\n`, '!border'],
|
||||
[`<div x-if="!border"></div>\n`, '!border'],
|
||||
|
||||
[`let notShadow = shadow \n`, 'shadow'],
|
||||
[`{ "foo": shadow.something + ""}\n`, 'shadow'],
|
||||
[`<div v-if="something && shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-else-if="something && shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-show="something && shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-if="shadow || shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-else-if="shadow || shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-show="shadow || shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-if="shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-else-if="shadow"></div>\n`, 'shadow'],
|
||||
[`<div v-show="shadow"></div>\n`, 'shadow'],
|
||||
[`<div x-if="shadow"></div>\n`, 'shadow'],
|
||||
[`<div style={{filter: 'drop-shadow(30px 10px 4px #4444dd)'}}/>\n`, 'shadow'],
|
||||
|
||||
// Next.js Image placeholder cases
|
||||
[`<Image placeholder="blur" src="/image.jpg" />`, 'blur'],
|
||||
[`<Image placeholder={'blur'} src="/image.jpg" />`, 'blur'],
|
||||
[`<Image placeholder={blur} src="/image.jpg" />`, 'blur'],
|
||||
|
||||
// https://github.com/tailwindlabs/tailwindcss/issues/17974
|
||||
['<div v-if="!duration">', '!duration'],
|
||||
['<div :active="!duration">', '!duration'],
|
||||
['<div :active="!visible">', '!visible'],
|
||||
|
||||
// Alpine/Livewire wire:…
|
||||
['<x-input.number required="foo" wire:model.blur="coins" />', 'blur'],
|
||||
|
||||
// Vue 3 events
|
||||
[`emit('blur', props.modelValue)\n`, 'blur'],
|
||||
[`$emit('blur', props.modelValue)\n`, 'blur'],
|
||||
|
||||
// JavaScript / TypeScript
|
||||
[`document.addEventListener('blur',handleBlur)`, 'blur'],
|
||||
[`document.addEventListener('blur', handleBlur)`, 'blur'],
|
||||
|
||||
[`function foo({ outline = true })`, 'outline'],
|
||||
[`function foo({ before = false, outline = true })`, 'outline'],
|
||||
[`function foo({before=false,outline=true })`, 'outline'],
|
||||
[`function foo({outline=true })`, 'outline'],
|
||||
// https://github.com/tailwindlabs/tailwindcss/issues/18675
|
||||
[
|
||||
// With default value
|
||||
`function foo({ size = "1.25rem", digit, outline = true, textClass = "", className = "" })`,
|
||||
'outline',
|
||||
],
|
||||
[
|
||||
// Without default value
|
||||
`function foo({ size = "1.25rem", digit, outline, textClass = "", className = "" })`,
|
||||
'outline',
|
||||
],
|
||||
[
|
||||
// As the last argument
|
||||
`function foo({ size = "1.25rem", digit, outline })`,
|
||||
'outline',
|
||||
],
|
||||
[
|
||||
// As the last argument, but there is techinically another `"` on the same line
|
||||
`function foo({ size = "1.25rem", digit, outline }): { return "foo" }`,
|
||||
'outline',
|
||||
],
|
||||
[
|
||||
// Tricky quote balancing
|
||||
`function foo({ before = "'", outline, after = "'" }): { return "foo" }`,
|
||||
'outline',
|
||||
],
|
||||
|
||||
[`function foo(blur, foo)`, 'blur'],
|
||||
[`function foo(blur,foo)`, 'blur'],
|
||||
])('does not replace classes in invalid positions #%#', async (example, candidate) => {
|
||||
expect(
|
||||
await migrateCandidate(designSystem, {}, candidate, {
|
||||
contents: example,
|
||||
|
|
@ -17,52 +99,5 @@ test('does not replace classes in invalid positions', async () => {
|
|||
end: example.indexOf(candidate) + candidate.length,
|
||||
}),
|
||||
).toEqual(candidate)
|
||||
}
|
||||
|
||||
await shouldNotReplace(`let notBorder = !border \n`)
|
||||
await shouldNotReplace(`{ "foo": !border.something + ""}\n`)
|
||||
await shouldNotReplace(`<div v-if="something && !border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-else-if="something && !border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-show="something && !border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-if="!border || !border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-else-if="!border || !border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-show="!border || !border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-if="!border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-else-if="!border"></div>\n`)
|
||||
await shouldNotReplace(`<div v-show="!border"></div>\n`)
|
||||
await shouldNotReplace(`<div x-if="!border"></div>\n`)
|
||||
|
||||
await shouldNotReplace(`let notShadow = shadow \n`, 'shadow')
|
||||
await shouldNotReplace(`{ "foo": shadow.something + ""}\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-if="something && shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-else-if="something && shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-show="something && shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-if="shadow || shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-else-if="shadow || shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-show="shadow || shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-if="shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-else-if="shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div v-show="shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(`<div x-if="shadow"></div>\n`, 'shadow')
|
||||
await shouldNotReplace(
|
||||
`<div style={{filter: 'drop-shadow(30px 10px 4px #4444dd)'}}/>\n`,
|
||||
'shadow',
|
||||
)
|
||||
|
||||
// Next.js Image placeholder cases
|
||||
await shouldNotReplace(`<Image placeholder="blur" src="/image.jpg" />`, 'blur')
|
||||
await shouldNotReplace(`<Image placeholder={'blur'} src="/image.jpg" />`, 'blur')
|
||||
await shouldNotReplace(`<Image placeholder={blur} src="/image.jpg" />`, 'blur')
|
||||
|
||||
// https://github.com/tailwindlabs/tailwindcss/issues/17974
|
||||
await shouldNotReplace('<div v-if="!duration">', '!duration')
|
||||
await shouldNotReplace('<div :active="!duration">', '!duration')
|
||||
await shouldNotReplace('<div :active="!visible">', '!visible')
|
||||
|
||||
// Alpine/Livewire wire:…
|
||||
await shouldNotReplace('<x-input.number required="foo" wire:model.blur="coins" />', 'blur')
|
||||
|
||||
// Vue 3 events
|
||||
await shouldNotReplace(`emit('blur', props.modelValue)\n`, 'blur')
|
||||
await shouldNotReplace(`$emit('blur', props.modelValue)\n`, 'blur')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
|
|||
import { DefaultMap } from '../../../../tailwindcss/src/utils/default-map'
|
||||
import * as version from '../../utils/version'
|
||||
|
||||
const QUOTES = ['"', "'", '`']
|
||||
const LOGICAL_OPERATORS = ['&&', '||', '?', '===', '==', '!=', '!==', '>', '>=', '<', '<=']
|
||||
const CONDITIONAL_TEMPLATE_SYNTAX = [
|
||||
// Vue
|
||||
|
|
@ -12,13 +11,16 @@ const CONDITIONAL_TEMPLATE_SYNTAX = [
|
|||
/v-show=['"]$/,
|
||||
/(?<!:?class)=['"]$/,
|
||||
|
||||
// JavaScript / TypeScript
|
||||
/addEventListener\(['"`]$/,
|
||||
|
||||
// Alpine
|
||||
/x-if=['"]$/,
|
||||
/x-show=['"]$/,
|
||||
/wire:[^\s]*?$/,
|
||||
]
|
||||
const NEXT_PLACEHOLDER_PROP = /placeholder=\{?['"]$/
|
||||
const VUE_3_EMIT = /\b\$?emit\(['"]$/
|
||||
const NEXT_PLACEHOLDER_PROP = /placeholder=\{?['"`]$/
|
||||
const VUE_3_EMIT = /\b\$?emit\(['"`]$/
|
||||
|
||||
export function isSafeMigration(
|
||||
rawCandidate: string,
|
||||
|
|
@ -138,8 +140,8 @@ export function isSafeMigration(
|
|||
}
|
||||
|
||||
// Heuristic: Require the candidate to be inside quotes
|
||||
let isQuoteBeforeCandidate = QUOTES.some((quote) => currentLineBeforeCandidate.includes(quote))
|
||||
let isQuoteAfterCandidate = QUOTES.some((quote) => currentLineAfterCandidate.includes(quote))
|
||||
let isQuoteBeforeCandidate = isMiddleOfString(currentLineBeforeCandidate)
|
||||
let isQuoteAfterCandidate = isMiddleOfString(currentLineAfterCandidate)
|
||||
if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
|
||||
return false
|
||||
}
|
||||
|
|
@ -210,3 +212,38 @@ const styleBlockRanges = new DefaultMap((source: string) => {
|
|||
ranges.push(startTag, endTag)
|
||||
}
|
||||
})
|
||||
|
||||
const BACKSLASH = 0x5c
|
||||
const DOUBLE_QUOTE = 0x22
|
||||
const SINGLE_QUOTE = 0x27
|
||||
const BACKTICK = 0x60
|
||||
|
||||
function isMiddleOfString(line: string): boolean {
|
||||
let currentQuote: number | null = null
|
||||
|
||||
for (let i = 0; i < line.length; i++) {
|
||||
let char = line.charCodeAt(i)
|
||||
switch (char) {
|
||||
// Escaped character, skip the next character
|
||||
case BACKSLASH:
|
||||
i++
|
||||
break
|
||||
|
||||
case SINGLE_QUOTE:
|
||||
case DOUBLE_QUOTE:
|
||||
case BACKTICK:
|
||||
// Found matching quote, we are outside of a string
|
||||
if (currentQuote === char) {
|
||||
currentQuote = null
|
||||
}
|
||||
|
||||
// Found a quote, we are inside a string
|
||||
else if (currentQuote === null) {
|
||||
currentQuote = char
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
return currentQuote !== null
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue