From 6dd2a58f99a0fb47d6feebc09ad89be213e2eda2 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Thu, 24 Oct 2024 14:18:34 +0200 Subject: [PATCH 1/2] Upgrade: Improve heuristics around important codemod --- CHANGELOG.md | 1 + integrations/upgrade/js-config.test.ts | 5 +- .../src/template/codemods/important.test.ts | 28 +++++--- .../src/template/codemods/important.ts | 71 ++++++++++++------- 4 files changed, 71 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14897da09111..c43dc481aa30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Ensure individual logical property utilities are sorted later than left/right pair utilities ([#14777](https://github.com/tailwindlabs/tailwindcss/pull/14777)) +- Don't migrate important modifiers when these are used inside conditionals of certain template languages (e.g. `
`) ([#14774](https://github.com/tailwindlabs/tailwindcss/pull/14774)) - _Upgrade (experimental)_: Ensure `@import` statements for relative CSS files are actually migrated to use relative path syntax ([#14769](https://github.com/tailwindlabs/tailwindcss/pull/14769)) ## [4.0.0-alpha.29] - 2024-10-23 diff --git a/integrations/upgrade/js-config.test.ts b/integrations/upgrade/js-config.test.ts index 9d2a6c5ee194..41fd83073264 100644 --- a/integrations/upgrade/js-config.test.ts +++ b/integrations/upgrade/js-config.test.ts @@ -123,9 +123,10 @@ test( @tailwind components; @tailwind utilities; `, + // prettier-ignore 'src/test.js': ts` export default { - shouldNotUse: !border.shouldUse, + 'shouldNotMigrate': !border.test + '', } `, 'node_modules/my-external-lib/src/template.html': html` @@ -275,7 +276,7 @@ test( --- src/test.js --- export default { - shouldNotUse: !border.shouldUse, + 'shouldNotMigrate': !border.test + '', } " `) diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts index 64f098b9bde4..56440f828116 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts @@ -38,16 +38,28 @@ test('does not match false positives', async () => { ).toEqual('!border') }) -test('does not match false positives with spaces at the end of the line', async () => { +test('does not match false positives', async () => { let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', { base: __dirname, }) - expect( - important(designSystem, {}, '!border', { - contents: `let notBorder = !border \n`, - start: 16, - end: 16 + '!border'.length, - }), - ).toEqual('!border') + function shouldNotDetect(example: string, candidate = '!border') { + expect( + important(designSystem, {}, candidate, { + contents: example, + start: example.indexOf(candidate), + end: example.indexOf(candidate) + candidate.length, + }), + ).toEqual('!border') + } + + shouldNotDetect(`let notBorder = !border \n`) + shouldNotDetect(`{ "foo": !border.something + ""}\n`) + shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) }) diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/important.ts b/packages/@tailwindcss-upgrade/src/template/codemods/important.ts index 4547905291ae..8aab7612c98f 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/important.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/important.ts @@ -3,6 +3,18 @@ import { parseCandidate } from '../../../../tailwindcss/src/candidate' import type { DesignSystem } from '../../../../tailwindcss/src/design-system' import { printCandidate } from '../candidates' +const QUOTES = ['"', "'", '`'] +const LOGICAL_OPERATORS = ['&&', '||', '===', '==', '!=', '!==', '>', '>=', '<', '<='] +const CONDITIONAL_TEMPLATE_SYNTAX = [ + // Vue + /v-if=['"]$/, + /v-show=['"]$/, + + // Alpine + /x-show=['"]$/, + /x-if=['"]$/, +] + // In v3 the important modifier `!` sits in front of the utility itself, not // before any of the variants. In v4, we want it to be at the end of the utility // so that it's always in the same location regardless of whether you used @@ -25,7 +37,7 @@ export function important( end: number }, ): string { - for (let candidate of parseCandidate(rawCandidate, designSystem)) { + nextCandidate: for (let candidate of parseCandidate(rawCandidate, designSystem)) { if (candidate.important && candidate.raw[candidate.raw.length - 1] !== '!') { // The important migration is one of the most broad migrations with a high // potential of matching false positives since `!` is a valid character in @@ -34,32 +46,54 @@ export function important( // on the side of caution and only migrate candidates that we are certain // are inside of a string. if (location) { - let isQuoteBeforeCandidate = false + let currentLineBeforeCandidate = '' for (let i = location.start - 1; i >= 0; i--) { let char = location.contents.at(i)! if (char === '\n') { break } - if (isQuote(char)) { - isQuoteBeforeCandidate = true - break - } + currentLineBeforeCandidate = char + currentLineBeforeCandidate } - - let isQuoteAfterCandidate = false + let currentLineAfterCandidate = '' for (let i = location.end; i < location.contents.length; i++) { let char = location.contents.at(i)! if (char === '\n') { break } - if (isQuote(char)) { - isQuoteAfterCandidate = true - break - } + currentLineAfterCandidate += char } + // Heuristics 1: Require the candidate to be inside quotes + let isQuoteBeforeCandidate = QUOTES.some((quote) => + currentLineBeforeCandidate.includes(quote), + ) + let isQuoteAfterCandidate = QUOTES.some((quote) => + currentLineAfterCandidate.includes(quote), + ) if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) { - continue + continue nextCandidate + } + + // Heuristics 2: Disallow object access immediately following the candidate + if (currentLineAfterCandidate[0] === '.') { + continue nextCandidate + } + + // Heuristics 3: Disallow logical operators proceeding or following the candidate + for (let operator of LOGICAL_OPERATORS) { + if ( + currentLineAfterCandidate.trim().startsWith(operator) || + currentLineBeforeCandidate.trim().endsWith(operator) + ) { + continue nextCandidate + } + } + + // Heuristics 4: Disallow conditional template syntax + for (let rule of CONDITIONAL_TEMPLATE_SYNTAX) { + if (rule.test(currentLineBeforeCandidate)) { + continue nextCandidate + } } } @@ -72,14 +106,3 @@ export function important( return rawCandidate } - -function isQuote(char: string) { - switch (char) { - case '"': - case "'": - case '`': - return true - default: - return false - } -} From 8abbc775838a5db671f485ab9ceb23f93716ba44 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Thu, 24 Oct 2024 17:19:13 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Adam Wathan --- CHANGELOG.md | 2 +- .../src/template/codemods/important.test.ts | 3 +++ .../src/template/codemods/important.ts | 11 ++++++----- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c43dc481aa30..d3fe6574524c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Ensure individual logical property utilities are sorted later than left/right pair utilities ([#14777](https://github.com/tailwindlabs/tailwindcss/pull/14777)) -- Don't migrate important modifiers when these are used inside conditionals of certain template languages (e.g. `
`) ([#14774](https://github.com/tailwindlabs/tailwindcss/pull/14774)) +- Don't migrate important modifiers inside conditional statements in Vue and Alpine (e.g. `
`) ([#14774](https://github.com/tailwindlabs/tailwindcss/pull/14774)) - _Upgrade (experimental)_: Ensure `@import` statements for relative CSS files are actually migrated to use relative path syntax ([#14769](https://github.com/tailwindlabs/tailwindcss/pull/14769)) ## [4.0.0-alpha.29] - 2024-10-23 diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts index 56440f828116..60d78aa53675 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts @@ -56,10 +56,13 @@ test('does not match false positives', async () => { shouldNotDetect(`let notBorder = !border \n`) shouldNotDetect(`{ "foo": !border.something + ""}\n`) shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) shouldNotDetect(`
\n`) shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) shouldNotDetect(`
\n`) shouldNotDetect(`
\n`) + shouldNotDetect(`
\n`) shouldNotDetect(`
\n`) shouldNotDetect(`
\n`) }) diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/important.ts b/packages/@tailwindcss-upgrade/src/template/codemods/important.ts index 8aab7612c98f..f9a8e2723c76 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/important.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/important.ts @@ -7,12 +7,13 @@ const QUOTES = ['"', "'", '`'] const LOGICAL_OPERATORS = ['&&', '||', '===', '==', '!=', '!==', '>', '>=', '<', '<='] const CONDITIONAL_TEMPLATE_SYNTAX = [ // Vue + /v-else-if=['"]$/, /v-if=['"]$/, /v-show=['"]$/, // Alpine - /x-show=['"]$/, /x-if=['"]$/, + /x-show=['"]$/, ] // In v3 the important modifier `!` sits in front of the utility itself, not @@ -63,7 +64,7 @@ export function important( currentLineAfterCandidate += char } - // Heuristics 1: Require the candidate to be inside quotes + // Heuristic 1: Require the candidate to be inside quotes let isQuoteBeforeCandidate = QUOTES.some((quote) => currentLineBeforeCandidate.includes(quote), ) @@ -74,12 +75,12 @@ export function important( continue nextCandidate } - // Heuristics 2: Disallow object access immediately following the candidate + // Heuristic 2: Disallow object access immediately following the candidate if (currentLineAfterCandidate[0] === '.') { continue nextCandidate } - // Heuristics 3: Disallow logical operators proceeding or following the candidate + // Heuristic 3: Disallow logical operators preceding or following the candidate for (let operator of LOGICAL_OPERATORS) { if ( currentLineAfterCandidate.trim().startsWith(operator) || @@ -89,7 +90,7 @@ export function important( } } - // Heuristics 4: Disallow conditional template syntax + // Heuristic 4: Disallow conditional template syntax for (let rule of CONDITIONAL_TEMPLATE_SYNTAX) { if (rule.test(currentLineBeforeCandidate)) { continue nextCandidate