From b722ebca37356c24a17c754112773bf8614fe222 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Thu, 24 Oct 2024 18:49:22 +0200 Subject: [PATCH] Upgrade: Ensure underscores in url() and var() are not escaped (#14778) This PR fixes an issue where currently a `theme()` function call inside an arbitrary value that used a dot in the key path: ```jsx let className = "ml-[theme(spacing[1.5])]" ``` Was causing issues when going though the codemod. The issue is that for candidates, we require `_` to be _escaped_, since otherwise they will be replaced with underscore. When going through the codemods, the above candidate would be translated to the following CSS variable access: ```js let className = "ml-[var(--spacing-1\_5))" ``` Because the underscore was escaped, we now have an invalid string inside a JavaScript file (as the `\` would escape inside the quoted string. To resolve this, we decided that this common case (as its used by the Tailwind CSS default theme) should work without escaping. In https://github.com/tailwindlabs/tailwindcss/pull/14776, we made the changes that CSS variables used via `var()` no longer unescape underscores. This PR extends that so that the Variant printer (that creates the serialized candidate representation after the codemods make changes) take this new encoding into account. This will result in the above example being translated into: ```js let className = "ml-[var(--spacing-1_5))" ``` With no more escaping. Nice! ## Test Plan I have added test for this to the kitchen-sink upgrade tests. Furthermore, to ensure this really works full-stack, I have updated the kitchen-sink test to _actually build the migrated project with Tailwind CSS v4_. After doing so, we can assert that we indeed have the right class name in the generated CSS. --------- Co-authored-by: Adam Wathan --- CHANGELOG.md | 1 + integrations/upgrade/index.test.ts | 25 +++++++- .../src/template/candidates.test.ts | 7 +++ .../src/template/candidates.ts | 57 ++++++++++++++++++- 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92e99cac1..479b3cc41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure third-party plugins with `exports` in their `package.json` are resolved correctly ([#14775](https://github.com/tailwindlabs/tailwindcss/pull/14775)) - Ensure underscores in the `url()` function are never unescaped ([#14776](https://github.com/tailwindlabs/tailwindcss/pull/14776)) - _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)) +- _Upgrade (experimental)_: Don't escape underscores when printing theme values migrated to CSS variables in arbitrary values (e.g. `m-[var(--spacing-1_5)]` instead of `m-[var(--spacing-1\_5)]`) ([#14778](https://github.com/tailwindlabs/tailwindcss/pull/14778)) ## [4.0.0-alpha.29] - 2024-10-23 diff --git a/integrations/upgrade/index.test.ts b/integrations/upgrade/index.test.ts index 83d1bc89e..567671786 100644 --- a/integrations/upgrade/index.test.ts +++ b/integrations/upgrade/index.test.ts @@ -1,5 +1,5 @@ import { expect } from 'vitest' -import { css, html, js, json, test } from '../utils' +import { candidate, css, html, js, json, test } from '../utils' test( `upgrades a v3 project to v4`, @@ -9,6 +9,9 @@ test( { "dependencies": { "@tailwindcss/upgrade": "workspace:^" + }, + "devDependencies": { + "@tailwindcss/cli": "workspace:^" } } `, @@ -20,7 +23,9 @@ test( `, 'src/index.html': html`

🤠👋

-
+
`, 'src/input.css': css` @tailwind base; @@ -42,7 +47,9 @@ test( " --- ./src/index.html ---

🤠👋

-
+
--- ./src/input.css --- @import 'tailwindcss'; @@ -92,6 +99,18 @@ test( expect(packageJson.dependencies).toMatchObject({ tailwindcss: expect.stringContaining('4.0.0'), }) + + // Ensure the v4 project compiles correctly + await exec('npx tailwindcss --input src/input.css --output dist/out.css') + + await fs.expectFileToContain('dist/out.css', [ + candidate`flex!`, + candidate`sm:block!`, + candidate`bg-linear-to-t`, + candidate`bg-[var(--my-red)]`, + candidate`max-w-[var(--breakpoint-md)]`, + candidate`ml-[var(--spacing-1\_5)`, + ]) }, ) diff --git a/packages/@tailwindcss-upgrade/src/template/candidates.test.ts b/packages/@tailwindcss-upgrade/src/template/candidates.test.ts index 296a5cc5e..4c310fe6f 100644 --- a/packages/@tailwindcss-upgrade/src/template/candidates.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/candidates.test.ts @@ -118,6 +118,13 @@ const candidates = [ // Keep spaces in strings ['content-["hello_world"]', 'content-["hello_world"]'], ['content-[____"hello_world"___]', 'content-["hello_world"]'], + + // Do not escape underscores for url() and CSS variable in var() + ['bg-[no-repeat_url(/image_13.png)]', 'bg-[no-repeat_url(/image_13.png)]'], + [ + 'bg-[var(--spacing-0_5,_var(--spacing-1_5,_3rem))]', + 'bg-[var(--spacing-0_5,_var(--spacing-1_5,_3rem))]', + ], ] const variants = [ diff --git a/packages/@tailwindcss-upgrade/src/template/candidates.ts b/packages/@tailwindcss-upgrade/src/template/candidates.ts index b97ff8ed0..b6ec3e682 100644 --- a/packages/@tailwindcss-upgrade/src/template/candidates.ts +++ b/packages/@tailwindcss-upgrade/src/template/candidates.ts @@ -187,9 +187,9 @@ function printArbitraryValue(input: string) { }) } + recursivelyEscapeUnderscores(ast) + return ValueParser.toCss(ast) - .replaceAll('_', String.raw`\_`) // Escape underscores to keep them as-is - .replaceAll(' ', '_') // Replace spaces with underscores } function simplifyArbitraryVariant(input: string) { @@ -213,3 +213,56 @@ function simplifyArbitraryVariant(input: string) { return input } + +function recursivelyEscapeUnderscores(ast: ValueParser.ValueAstNode[]) { + for (let node of ast) { + switch (node.kind) { + case 'function': { + if (node.value === 'url' || node.value.endsWith('_url')) { + // Don't decode underscores in url() but do decode the function name + node.value = escapeUnderscore(node.value) + break + } + + if ( + node.value === 'var' || + node.value.endsWith('_var') || + node.value === 'theme' || + node.value.endsWith('_theme') + ) { + // Don't decode underscores in the first argument of var() and theme() + // but do decode the function name + node.value = escapeUnderscore(node.value) + for (let i = 0; i < node.nodes.length; i++) { + if (i == 0 && node.nodes[i].kind === 'word') { + continue + } + recursivelyEscapeUnderscores([node.nodes[i]]) + } + break + } + + node.value = escapeUnderscore(node.value) + recursivelyEscapeUnderscores(node.nodes) + break + } + case 'separator': + case 'word': { + node.value = escapeUnderscore(node.value) + break + } + default: + never(node) + } + } +} + +function never(value: never): never { + throw new Error(`Unexpected value: ${value}`) +} + +function escapeUnderscore(value: string): string { + return value + .replaceAll('_', String.raw`\_`) // Escape underscores to keep them as-is + .replaceAll(' ', '_') // Replace spaces with underscores +}