From 30be24b29f78cccc8016eacddd733983f23e7e96 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 12 Aug 2025 15:48:35 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../template/is-safe-migration.test.ts | 137 +++++++++++------- .../codemods/template/is-safe-migration.ts | 47 +++++- 3 files changed, 129 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3971ac3ee..bfb626528 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts index 5ade133e0..f62854801 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts @@ -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'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + [`
\n`, '!border'], + + [`let notShadow = shadow \n`, 'shadow'], + [`{ "foo": shadow.something + ""}\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + [`
\n`, 'shadow'], + + // Next.js Image placeholder cases + [``, 'blur'], + [``, 'blur'], + [``, 'blur'], + + // https://github.com/tailwindlabs/tailwindcss/issues/17974 + ['
', '!duration'], + ['
', '!duration'], + ['
', '!visible'], + + // Alpine/Livewire wire:… + ['', '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(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - await shouldNotReplace(`
\n`) - - await shouldNotReplace(`let notShadow = shadow \n`, 'shadow') - await shouldNotReplace(`{ "foo": shadow.something + ""}\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace(`
\n`, 'shadow') - await shouldNotReplace( - `
\n`, - 'shadow', - ) - - // Next.js Image placeholder cases - await shouldNotReplace(``, 'blur') - await shouldNotReplace(``, 'blur') - await shouldNotReplace(``, 'blur') - - // https://github.com/tailwindlabs/tailwindcss/issues/17974 - await shouldNotReplace('
', '!duration') - await shouldNotReplace('
', '!duration') - await shouldNotReplace('
', '!visible') - - // Alpine/Livewire wire:… - await shouldNotReplace('', 'blur') - - // Vue 3 events - await shouldNotReplace(`emit('blur', props.modelValue)\n`, 'blur') - await shouldNotReplace(`$emit('blur', props.modelValue)\n`, 'blur') + }) }) diff --git a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts index 7ac24d645..e4ec86b28 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts @@ -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=['"]$/, /(? 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 +}