Skip to content

Upgrade: Improve heuristics around important codemod #14774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 inside conditional statements in Vue and Alpine (e.g. `<div v-if="!border" />`) ([#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
Expand Down
5 changes: 3 additions & 2 deletions integrations/upgrade/js-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -275,7 +276,7 @@ test(

--- src/test.js ---
export default {
shouldNotUse: !border.shouldUse,
'shouldNotMigrate': !border.test + '',
}
"
`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,31 @@ 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(`<div v-if="something && !border"></div>\n`)
shouldNotDetect(`<div v-else-if="something && !border"></div>\n`)
shouldNotDetect(`<div v-show="something && !border"></div>\n`)
shouldNotDetect(`<div v-if="!border || !border"></div>\n`)
shouldNotDetect(`<div v-else-if="!border || !border"></div>\n`)
shouldNotDetect(`<div v-show="!border || !border"></div>\n`)
shouldNotDetect(`<div v-if="!border"></div>\n`)
shouldNotDetect(`<div v-else-if="!border"></div>\n`)
shouldNotDetect(`<div v-show="!border"></div>\n`)
shouldNotDetect(`<div x-if="!border"></div>\n`)
})
72 changes: 48 additions & 24 deletions packages/@tailwindcss-upgrade/src/template/codemods/important.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,19 @@ 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-else-if=['"]$/,
/v-if=['"]$/,
/v-show=['"]$/,

// Alpine
/x-if=['"]$/,
/x-show=['"]$/,
]

// 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
Expand All @@ -25,7 +38,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
Expand All @@ -34,32 +47,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
}

// Heuristic 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
}

// Heuristic 2: Disallow object access immediately following the candidate
if (currentLineAfterCandidate[0] === '.') {
continue nextCandidate
}

// Heuristic 3: Disallow logical operators preceding or following the candidate
for (let operator of LOGICAL_OPERATORS) {
if (
currentLineAfterCandidate.trim().startsWith(operator) ||
currentLineBeforeCandidate.trim().endsWith(operator)
) {
continue nextCandidate
}
}

// Heuristic 4: Disallow conditional template syntax
for (let rule of CONDITIONAL_TEMPLATE_SYNTAX) {
if (rule.test(currentLineBeforeCandidate)) {
continue nextCandidate
}
}
}

Expand All @@ -72,14 +107,3 @@ export function important(

return rawCandidate
}

function isQuote(char: string) {
switch (char) {
case '"':
case "'":
case '`':
return true
default:
return false
}
}