From 61727e63d0d85cd0e5f224eb1f8051f6dfe70ca6 Mon Sep 17 00:00:00 2001 From: Brandon McConnell Date: Tue, 6 May 2025 03:27:05 -0400 Subject: [PATCH 01/11] Don't `decodeArbitraryValue` variable shorthand syntax in modifiers --- packages/tailwindcss/src/candidate.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index aff6d74dc116..7a29d1626c4d 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -523,7 +523,7 @@ function parseModifier(modifier: string): CandidateModifier | null { } if (modifier[0] === '(' && modifier[modifier.length - 1] === ')') { - let arbitraryValue = decodeArbitraryValue(modifier.slice(1, -1)) + let arbitraryValue = modifier.slice(1, -1) // Values can't contain `;` or `}` characters at the top-level. if (!isValidArbitrary(arbitraryValue)) return null From 58a9360c6ab8f2f80d39aadadc05130275112248 Mon Sep 17 00:00:00 2001 From: Brandon McConnell Date: Tue, 6 May 2025 03:35:03 -0400 Subject: [PATCH 02/11] Add test --- packages/tailwindcss/src/candidate.test.ts | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/tailwindcss/src/candidate.test.ts b/packages/tailwindcss/src/candidate.test.ts index 3c54bef7a20d..fa3acec62295 100644 --- a/packages/tailwindcss/src/candidate.test.ts +++ b/packages/tailwindcss/src/candidate.test.ts @@ -1087,6 +1087,7 @@ it('should parse a utility with an implicit variable as the modifier using the s let utilities = new Utilities() utilities.functional('bg', () => []) + // Standard case (no underscores) expect(run('bg-red-500/(--value)', { utilities })).toMatchInlineSnapshot(` [ { @@ -1107,6 +1108,28 @@ it('should parse a utility with an implicit variable as the modifier using the s }, ] `) + + // Should preserve underscores + expect(run('bg-red-500/(--with_underscore)', { utilities })).toMatchInlineSnapshot(` + [ + { + "important": false, + "kind": "functional", + "modifier": { + "kind": "arbitrary", + "value": "var(--with_underscore)", + }, + "raw": "bg-red-500/(--with_underscore)", + "root": "bg", + "value": { + "fraction": null, + "kind": "named", + "value": "red-500", + }, + "variants": [], + }, + ] + `) }) it('should not parse a utility with an implicit invalid variable as the modifier using the shorthand', () => { From 86278af856bd8d06bff2223fad11e5f6178d47f4 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 11:57:33 +0200 Subject: [PATCH 03/11] add a few more tests These tests include CSS variables with underscores that should not be replaced, but also fallback values that should be replaced. --- packages/tailwindcss/src/candidate.test.ts | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/packages/tailwindcss/src/candidate.test.ts b/packages/tailwindcss/src/candidate.test.ts index fa3acec62295..1d2a159e3545 100644 --- a/packages/tailwindcss/src/candidate.test.ts +++ b/packages/tailwindcss/src/candidate.test.ts @@ -1130,6 +1130,53 @@ it('should parse a utility with an implicit variable as the modifier using the s }, ] `) + + // Should remove underscores in fallback values + expect(run('bg-red-500/(--with_underscore,fallback_value)', { utilities })) + .toMatchInlineSnapshot(` + [ + { + "important": false, + "kind": "functional", + "modifier": { + "kind": "arbitrary", + "value": "var(--with_underscore,fallback value)", + }, + "raw": "bg-red-500/(--with_underscore,fallback_value)", + "root": "bg", + "value": { + "fraction": null, + "kind": "named", + "value": "red-500", + }, + "variants": [], + }, + ] + `) + + // Should keep underscores in the CSS variable itself, but remove underscores + // in fallback values + expect(run('bg-(--a_b,c_d_var(--e_f,g_h))/(--i_j,k_l_var(--m_n,o_p))', { utilities })) + .toMatchInlineSnapshot(` + [ + { + "important": false, + "kind": "functional", + "modifier": { + "kind": "arbitrary", + "value": "var(--i_j,k l var(--m_n,o p))", + }, + "raw": "bg-(--a_b,c_d_var(--e_f,g_h))/(--i_j,k_l_var(--m_n,o_p))", + "root": "bg", + "value": { + "dataType": null, + "kind": "arbitrary", + "value": "var(--a_b,c d var(--e_f,g h))", + }, + "variants": [], + }, + ] + `) }) it('should not parse a utility with an implicit invalid variable as the modifier using the shorthand', () => { From 8fd6eedbd5139718ac84bb753758558958ba356c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 11:58:10 +0200 Subject: [PATCH 04/11] =?UTF-8?q?arbitrary=20modifier=20shorthand=20should?= =?UTF-8?q?=20be=20wrapped=20in=20`var(=E2=80=A6)`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This now has the same behavior as arbitrary value shorthand where `(--my-value)` is wrapped in a `var(--my-value)` behind the scenes. Doing this, allows us to perform 2 things: 1. We can check early if the value is an actual CSS variable — aka, an arbitrary modifier shorthand starts and ends with `(` and `)` respectively and starts with `--` inside the parens. E.g.: `(--…)` 2. Wrapping in `var(…)` allows us to use the `decodeArbitraryValue` which handles CSS variables (and fallbacks) already for us. --- packages/tailwindcss/src/candidate.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index 7a29d1626c4d..f7f61f4603d2 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -523,7 +523,17 @@ function parseModifier(modifier: string): CandidateModifier | null { } if (modifier[0] === '(' && modifier[modifier.length - 1] === ')') { - let arbitraryValue = modifier.slice(1, -1) + // Drop the `(` and `)` characters + modifier = modifier.slice(1, -1) + + // A modifier with `(…)` should always start with `--` since it + // represents a CSS variable. + if (modifier[0] !== '-' && modifier[1] !== '-') return null + + // Wrap the value in `var(…)` to ensure that it is a CSS variable. + modifier = `var(${modifier})` + + let arbitraryValue = decodeArbitraryValue(modifier) // Values can't contain `;` or `}` characters at the top-level. if (!isValidArbitrary(arbitraryValue)) return null @@ -532,12 +542,9 @@ function parseModifier(modifier: string): CandidateModifier | null { // ^^ if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null - // Arbitrary values must start with `--` since it represents a CSS variable. - if (arbitraryValue[0] !== '-' && arbitraryValue[1] !== '-') return null - return { kind: 'arbitrary', - value: `var(${arbitraryValue})`, + value: arbitraryValue, } } From 32b7abdf841aab1950e964278c7cbaf03f495c12 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 12:07:13 +0200 Subject: [PATCH 05/11] move empty check up --- packages/tailwindcss/src/candidate.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index f7f61f4603d2..1f9b8d897b23 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -530,6 +530,14 @@ function parseModifier(modifier: string): CandidateModifier | null { // represents a CSS variable. if (modifier[0] !== '-' && modifier[1] !== '-') return null + // Trim the modifier to remove any leading or trailing whitespace + modifier = modifier.trim() + + // Empty arbitrary values are invalid. E.g.: `data-(--):` + // ^^ + // Note: we already validated that the `modifier` starts with `--` + if (modifier.length === 2) return null + // Wrap the value in `var(…)` to ensure that it is a CSS variable. modifier = `var(${modifier})` @@ -538,10 +546,6 @@ function parseModifier(modifier: string): CandidateModifier | null { // Values can't contain `;` or `}` characters at the top-level. if (!isValidArbitrary(arbitraryValue)) return null - // Empty arbitrary values are invalid. E.g.: `data-():` - // ^^ - if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null - return { kind: 'arbitrary', value: arbitraryValue, From 803c5111e2363a245b7492aa9a567e1a7170918f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 12:12:08 +0200 Subject: [PATCH 06/11] fix De Morgan's law Essentially if we had `value[0] !== '-' && value[1] !== '-'`, a value such as `_-` would pass. E.g.: `var(_--foo)` Instead, we have to ensure that both the first and second character are `-` characters. Alternatively, we could write `!(value[0] === '-' && value[1] === '-')`, but applying De Morgan's law means: `!(value[0] === '-' && value[1] === '-')` Becomes: `value[0] !== '-' || value[1] !== '-'` --- packages/tailwindcss/src/candidate.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index 1f9b8d897b23..5c92e788bdbc 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -411,7 +411,7 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter // An arbitrary value with `(…)` should always start with `--` since it // represents a CSS variable. - if (value[0] !== '-' && value[1] !== '-') return + if (value[0] !== '-' || value[1] !== '-') return roots = [[root, dataType === null ? `[var(${value})]` : `[${dataType}:var(${value})]`]] } @@ -528,7 +528,7 @@ function parseModifier(modifier: string): CandidateModifier | null { // A modifier with `(…)` should always start with `--` since it // represents a CSS variable. - if (modifier[0] !== '-' && modifier[1] !== '-') return null + if (modifier[0] !== '-' || modifier[1] !== '-') return null // Trim the modifier to remove any leading or trailing whitespace modifier = modifier.trim() @@ -690,7 +690,7 @@ export function parseVariant(variant: string, designSystem: DesignSystem): Varia if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null // Arbitrary values must start with `--` since it represents a CSS variable. - if (arbitraryValue[0] !== '-' && arbitraryValue[1] !== '-') return null + if (arbitraryValue[0] !== '-' || arbitraryValue[1] !== '-') return null return { kind: 'functional', @@ -1041,7 +1041,7 @@ function recursivelyEscapeUnderscores(ast: ValueParser.ValueAstNode[]) { case 'word': { // Dashed idents and variables `var(--my-var)` and `--my-var` should not // have underscores escaped - if (node.value[0] !== '-' && node.value[1] !== '-') { + if (node.value[0] !== '-' || node.value[1] !== '-') { node.value = escapeUnderscore(node.value) } break From 4b4417ec36541301f46996a0e2e99bb7b2537f36 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 12:42:10 +0200 Subject: [PATCH 07/11] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef34a1a71862..4364d8059c23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure negative arbitrary `scale` values generate negative values ([#17831](https://github.com/tailwindlabs/tailwindcss/pull/17831)) - Fix HAML extraction with embedded Ruby ([#17846](https://github.com/tailwindlabs/tailwindcss/pull/17846)) - Don't scan files for utilities when using `@reference` ([#17836](https://github.com/tailwindlabs/tailwindcss/pull/17836)) +- Fix incorrectly replacing `_` with ` ` in arbitrary modifier shorthand `bg-red-500/(--my_opacity)` ([#17889](https://github.com/tailwindlabs/tailwindcss/pull/17889)) ## [4.1.5] - 2025-04-30 From 4fd14f3c967f7abe20d4dd9716a5636893b15398 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 13:51:36 +0200 Subject: [PATCH 08/11] add more validation related tests --- packages/tailwindcss/src/candidate.test.ts | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/tailwindcss/src/candidate.test.ts b/packages/tailwindcss/src/candidate.test.ts index 1d2a159e3545..43b92d8904ca 100644 --- a/packages/tailwindcss/src/candidate.test.ts +++ b/packages/tailwindcss/src/candidate.test.ts @@ -1179,6 +1179,48 @@ it('should parse a utility with an implicit variable as the modifier using the s `) }) +it('should not parse an invalid arbitrary shorthand modifier', () => { + let utilities = new Utilities() + utilities.functional('bg', () => []) + + // Completely empty + expect(run('bg-red-500/()', { utilities })).toMatchInlineSnapshot(`[]`) + + // Invalid due to leading spaces + expect(run('bg-red-500/(_--)', { utilities })).toMatchInlineSnapshot(`[]`) + expect(run('bg-red-500/(_--x)', { utilities })).toMatchInlineSnapshot(`[]`) + + // Invalid due to leading spaces + expect(run('bg-red-500/(_--)', { utilities })).toMatchInlineSnapshot(`[]`) + expect(run('bg-red-500/(_--x)', { utilities })).toMatchInlineSnapshot(`[]`) + + // Invalid due to top-level `;` or `}` characters + expect(run('bg-red-500/(--x;--y)', { utilities })).toMatchInlineSnapshot(`[]`) + expect(run('bg-red-500/(--x:{foo:bar})', { utilities })).toMatchInlineSnapshot(`[]`) + + // Valid, but ensuring that we didn't make an off-by-one error + expect(run('bg-red-500/(--x)', { utilities })).toMatchInlineSnapshot(` + [ + { + "important": false, + "kind": "functional", + "modifier": { + "kind": "arbitrary", + "value": "var(--x)", + }, + "raw": "bg-red-500/(--x)", + "root": "bg", + "value": { + "fraction": null, + "kind": "named", + "value": "red-500", + }, + "variants": [], + }, + ] + `) +}) + it('should not parse a utility with an implicit invalid variable as the modifier using the shorthand', () => { let utilities = new Utilities() utilities.functional('bg', () => []) From 6b715504adab9593d50250d5daf33de06d283895 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 15:06:26 +0200 Subject: [PATCH 09/11] move checks around --- packages/tailwindcss/src/candidate.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index 5c92e788bdbc..6f77292575d5 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -530,22 +530,14 @@ function parseModifier(modifier: string): CandidateModifier | null { // represents a CSS variable. if (modifier[0] !== '-' || modifier[1] !== '-') return null - // Trim the modifier to remove any leading or trailing whitespace - modifier = modifier.trim() - - // Empty arbitrary values are invalid. E.g.: `data-(--):` - // ^^ - // Note: we already validated that the `modifier` starts with `--` - if (modifier.length === 2) return null + // Values can't contain `;` or `}` characters at the top-level. + if (!isValidArbitrary(modifier)) return null - // Wrap the value in `var(…)` to ensure that it is a CSS variable. + // Wrap the value in `var(…)` to ensure that it is a valid CSS variable. modifier = `var(${modifier})` let arbitraryValue = decodeArbitraryValue(modifier) - // Values can't contain `;` or `}` characters at the top-level. - if (!isValidArbitrary(arbitraryValue)) return null - return { kind: 'arbitrary', value: arbitraryValue, From 9e86961e0b8535e0966035a429982767a6e2c340 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 15:21:08 +0200 Subject: [PATCH 10/11] add tests for arbitrary value shorthands In value position instead of modifier position --- packages/tailwindcss/src/candidate.test.ts | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/tailwindcss/src/candidate.test.ts b/packages/tailwindcss/src/candidate.test.ts index 43b92d8904ca..4355071f2aac 100644 --- a/packages/tailwindcss/src/candidate.test.ts +++ b/packages/tailwindcss/src/candidate.test.ts @@ -1221,6 +1221,45 @@ it('should not parse an invalid arbitrary shorthand modifier', () => { `) }) +it('should not parse an invalid arbitrary shorthand value', () => { + let utilities = new Utilities() + utilities.functional('bg', () => []) + + // Completely empty + expect(run('bg-()', { utilities })).toMatchInlineSnapshot(`[]`) + + // Invalid due to leading spaces + expect(run('bg-(_--)', { utilities })).toMatchInlineSnapshot(`[]`) + expect(run('bg-(_--x)', { utilities })).toMatchInlineSnapshot(`[]`) + + // Invalid due to leading spaces + expect(run('bg-(_--)', { utilities })).toMatchInlineSnapshot(`[]`) + expect(run('bg-(_--x)', { utilities })).toMatchInlineSnapshot(`[]`) + + // Invalid due to top-level `;` or `}` characters + expect(run('bg-(--x;--y)', { utilities })).toMatchInlineSnapshot(`[]`) + expect(run('bg-(--x:{foo:bar})', { utilities })).toMatchInlineSnapshot(`[]`) + + // Valid, but ensuring that we didn't make an off-by-one error + expect(run('bg-(--x)', { utilities })).toMatchInlineSnapshot(` + [ + { + "important": false, + "kind": "functional", + "modifier": null, + "raw": "bg-(--x)", + "root": "bg", + "value": { + "dataType": null, + "kind": "arbitrary", + "value": "var(--x)", + }, + "variants": [], + }, + ] + `) +}) + it('should not parse a utility with an implicit invalid variable as the modifier using the shorthand', () => { let utilities = new Utilities() utilities.functional('bg', () => []) From ebc1da8310fce44692da0a0b0e460b457664224c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 6 May 2025 15:21:31 +0200 Subject: [PATCH 11/11] ensure we validate the variable shorthand E.g.: `bg-(--foo)` --- packages/tailwindcss/src/candidate.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index 6f77292575d5..5a5704989a30 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -413,6 +413,9 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter // represents a CSS variable. if (value[0] !== '-' || value[1] !== '-') return + // Values can't contain `;` or `}` characters at the top-level. + if (!isValidArbitrary(value)) return + roots = [[root, dataType === null ? `[var(${value})]` : `[${dataType}:var(${value})]`]] }