Skip to content

Commit

Permalink
Ensure it's safe to perform suffix-less migrations (#14979)
Browse files Browse the repository at this point in the history
This PR makes sure that migrations from suffix-less candidates (e.g.:
`rounded`, `blur`, `shadow`) are safe to be migrated.

In some code snippets that's not always the case.

Given the following code snippet:
```tsx
type Star = [
  x: number,
  y: number,
  dim?: boolean,
  blur?: boolean,
  rounded?: boolean,
  shadow?: boolean,
]

function Star({ point: [cx, cy, dim, blur, rounded, shadow] }: { point: Star }) {
  return <svg class="rounded shadow blur" filter={blur ? 'url(…)' : undefined} />
}
```

Without this change, it would result in:
```tsx
type Star = [
  x: number,
  y: number,
  dim?: boolean,
  blur-sm?: boolean,
  rounded-sm?: boolean,
  shadow-sm?: boolean,
]

function Star({ point: [cx, cy, dim, blur-sm, rounded-sm, shadow-sm] }: { point: Star }) {
  return <svg class="rounded-sm shadow-sm blur-sm" filter={blur-sm ? 'url(…)' : undefined} />
}
```

But with this change, it results in:
```tsx
type Star = [
  x: number,
  y: number,
  dim?: boolean,
  blur?: boolean,
  rounded?: boolean,
  shadow?: boolean,
]

function Star({ point: [cx, cy, dim, blur, rounded, shadow] }: { point: Star }) {
  return <svg class="rounded-sm shadow-sm blur-sm" filter={blur ? 'url(…)' : undefined} />
}
```

Notice how the classes inside the `class` attribute _are_ converted, but
the ones in the types or as part of the JavaScript code (e.g.:
`filter={blur ? 'url(…)' : undefined}`) are not.

---------

Co-authored-by: Philipp Spiess <[email protected]>
  • Loading branch information
RobinMalfait and philipp-spiess authored Nov 13, 2024
1 parent ac11740 commit c63f01b
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 71 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- _Upgrade (experimental)_: Do not migrate legacy classes with custom values ([#14976](https://github.com/tailwindlabs/tailwindcss/pull/14976))
- _Upgrade (experimental)_: Ensure it's safe to migrate `blur`, `rounded`, or `shadow` ([#14979](https://github.com/tailwindlabs/tailwindcss/pull/14979))

## [4.0.0-alpha.33] - 2024-11-11

Expand Down
94 changes: 93 additions & 1 deletion integrations/upgrade/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from 'vitest'
import { candidate, css, html, js, json, test } from '../utils'
import { candidate, css, html, js, json, test, ts } from '../utils'

test(
'error when no CSS file with @tailwind is used',
Expand Down Expand Up @@ -1747,3 +1747,95 @@ test(
`)
},
)

test(
'make suffix-less migrations safe (e.g.: `blur`, `rounded`, `shadow`)',
{
fs: {
'package.json': json`
{
"dependencies": {
"tailwindcss": "^3.4.14",
"@tailwindcss/upgrade": "workspace:^"
},
"devDependencies": {
"prettier-plugin-tailwindcss": "0.5.0"
}
}
`,
'tailwind.config.js': js`
module.exports = {
content: ['./*.{html,tsx}'],
}
`,
'index.css': css`
@tailwind base;
@tailwind components;
@tailwind utilities;
`,
'index.html': html`
<div class="rounded blur shadow"></div>
`,
'example-component.tsx': ts`
type Star = [
x: number,
y: number,
dim?: boolean,
blur?: boolean,
rounded?: boolean,
shadow?: boolean,
]
function Star({ point: [cx, cy, dim, blur, rounded, shadow] }: { point: Star }) {
return <svg class="rounded shadow blur" filter={blur ? 'url(…)' : undefined} />
}
`,
},
},
async ({ fs, exec }) => {
await exec('npx @tailwindcss/upgrade --force')

// Files should not be modified
expect(await fs.dumpFiles('./*.{js,css,html,tsx}')).toMatchInlineSnapshot(`
"
--- index.html ---
<div class="rounded-sm blur-sm shadow-sm"></div>
--- index.css ---
@import 'tailwindcss';
/*
The default border color has changed to \`currentColor\` in Tailwind CSS v4,
so we've added these compatibility styles to make sure everything still
looks the same as it did with Tailwind CSS v3.
If we ever want to remove these styles, we need to add an explicit border
color utility to any element that depends on these defaults.
*/
@layer base {
*,
::after,
::before,
::backdrop,
::file-selector-button {
border-color: var(--color-gray-200, currentColor);
}
}
--- example-component.tsx ---
type Star = [
x: number,
y: number,
dim?: boolean,
blur?: boolean,
rounded?: boolean,
shadow?: boolean,
]
function Star({ point: [cx, cy, dim, blur, rounded, shadow] }: { point: Star }) {
return <svg class="rounded-sm shadow-sm blur-sm" filter={blur ? 'url(…)' : undefined} />
}
"
`)
},
)
66 changes: 3 additions & 63 deletions packages/@tailwindcss-upgrade/src/template/codemods/important.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,7 @@ import type { Config } from 'tailwindcss'
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=['"]$/,
]
import { isSafeMigration } from '../is-safe-migration'

// 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
Expand Down Expand Up @@ -46,56 +34,8 @@ export function important(
// with v3 in that it can read `!` in the front of the utility too, we err
// on the side of caution and only migrate candidates that we are certain
// are inside of a string.
if (location) {
let currentLineBeforeCandidate = ''
for (let i = location.start - 1; i >= 0; i--) {
let char = location.contents.at(i)!
if (char === '\n') {
break
}
currentLineBeforeCandidate = char + currentLineBeforeCandidate
}
let currentLineAfterCandidate = ''
for (let i = location.end; i < location.contents.length; i++) {
let char = location.contents.at(i)!
if (char === '\n') {
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 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
}
}
if (location && !isSafeMigration(location)) {
continue nextCandidate
}

// The printCandidate function will already put the exclamation mark in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { Config } from 'tailwindcss'
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
import { DefaultMap } from '../../../../tailwindcss/src/utils/default-map'
import { printCandidate } from '../candidates'
import { isSafeMigration } from '../is-safe-migration'

const __filename = url.fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)
Expand Down Expand Up @@ -56,6 +57,11 @@ export async function legacyClasses(
designSystem: DesignSystem,
_userConfig: Config,
rawCandidate: string,
location?: {
contents: string
start: number
end: number
},
): Promise<string> {
// Ensure the "old" classes exist as static utilities to make the migration
// easier because the "root" will point to the full class.
Expand All @@ -70,13 +76,14 @@ export async function legacyClasses(

for (let candidate of designSystem.parseCandidate(rawCandidate)) {
if (candidate.kind === 'static' && Object.hasOwn(LEGACY_CLASS_MAP, candidate.root)) {
let newRoot = LEGACY_CLASS_MAP[candidate.root as keyof typeof LEGACY_CLASS_MAP]

if (location && !candidate.root.includes('-') && !isSafeMigration(location)) {
continue
}

let fromThemeKey = THEME_KEYS[candidate.root as keyof typeof THEME_KEYS]
let toThemeKey =
THEME_KEYS[
LEGACY_CLASS_MAP[
candidate.root as keyof typeof LEGACY_CLASS_MAP
] as keyof typeof THEME_KEYS
]
let toThemeKey = THEME_KEYS[newRoot as keyof typeof THEME_KEYS]

if (fromThemeKey && toThemeKey) {
// Migrating something that resolves to a value in the theme.
Expand Down Expand Up @@ -104,7 +111,7 @@ export async function legacyClasses(

return printCandidate(designSystem, {
...candidate,
root: LEGACY_CLASS_MAP[candidate.root as keyof typeof LEGACY_CLASS_MAP],
root: newRoot,
})
}
}
Expand Down
62 changes: 62 additions & 0 deletions packages/@tailwindcss-upgrade/src/template/is-safe-migration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
const QUOTES = ['"', "'", '`']
const LOGICAL_OPERATORS = ['&&', '||', '?', '===', '==', '!=', '!==', '>', '>=', '<', '<=']
const CONDITIONAL_TEMPLATE_SYNTAX = [
// Vue
/v-else-if=['"]$/,
/v-if=['"]$/,
/v-show=['"]$/,

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

export function isSafeMigration(location: { contents: string; start: number; end: number }) {
let currentLineBeforeCandidate = ''
for (let i = location.start - 1; i >= 0; i--) {
let char = location.contents.at(i)!
if (char === '\n') {
break
}
currentLineBeforeCandidate = char + currentLineBeforeCandidate
}
let currentLineAfterCandidate = ''
for (let i = location.end; i < location.contents.length; i++) {
let char = location.contents.at(i)!
if (char === '\n') {
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) {
return false
}

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

// 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)
) {
return false
}
}

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

return true
}

0 comments on commit c63f01b

Please sign in to comment.