diff --git a/CHANGELOG.md b/CHANGELOG.md index eba00c7598a7..e2bb26e9a785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Use single drop shadow values instead of multiple ([#15056](https://github.com/tailwindlabs/tailwindcss/pull/15056)) +- Do not parse invalid candidates with empty arbitrary values ([#15055](https://github.com/tailwindlabs/tailwindcss/pull/15055)) ## [4.0.0-alpha.35] - 2024-11-20 diff --git a/crates/oxide/src/parser.rs b/crates/oxide/src/parser.rs index b32f36beaaa9..8e659a258225 100644 --- a/crates/oxide/src/parser.rs +++ b/crates/oxide/src/parser.rs @@ -538,14 +538,17 @@ impl<'a> Extractor<'a> { return ParseAction::Consume; } - if let Arbitrary::Brackets { start_idx } = self.arbitrary { + if let Arbitrary::Brackets { start_idx: _ } = self.arbitrary { trace!("Arbitrary::End\t"); self.arbitrary = Arbitrary::None; - if self.cursor.pos - start_idx == 1 { - // We have an empty arbitrary value, which is not allowed - return ParseAction::Skip; - } + // TODO: This is temporarily disabled such that the upgrade tool can work + // with legacy arbitrary values. This will be re-enabled in the future (or + // with a flag) + // if self.cursor.pos - start_idx == 1 { + // // We have an empty arbitrary value, which is not allowed + // return ParseAction::Skip; + // } } } @@ -1517,4 +1520,23 @@ mod test { assert_eq!(candidates, vec![("div", 1), ("class", 5), ("flex", 12),]); } + + #[test] + fn empty_arbitrary_values_are_allowed_for_codemods() { + let candidates = run( + r#"
"#, + false, + ); + assert_eq!( + candidates, + vec![ + "div", + "class", + "group-[]:flex", + "group-[]/name:flex", + "peer-[]:flex", + "peer-[]/name:flex" + ] + ); + } } diff --git a/integrations/upgrade/index.test.ts b/integrations/upgrade/index.test.ts index 1d91788d3e7e..438d64cf154e 100644 --- a/integrations/upgrade/index.test.ts +++ b/integrations/upgrade/index.test.ts @@ -68,7 +68,7 @@ test( 'src/index.html': html`

🤠👋

@@ -100,7 +100,7 @@ test( --- ./src/index.html ---

🤠👋

@@ -194,7 +194,7 @@ test( `, 'src/index.html': html`
`, 'src/input.css': css` @@ -215,7 +215,7 @@ test( " --- ./src/index.html ---
--- ./src/input.css --- diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.test.ts new file mode 100644 index 000000000000..b451fbf86b8e --- /dev/null +++ b/packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.test.ts @@ -0,0 +1,37 @@ +import { __unstable__loadDesignSystem } from '@tailwindcss/node' +import { expect, test } from 'vitest' +import { handleEmptyArbitraryValues } from './handle-empty-arbitrary-values' +import { prefix } from './prefix' + +test.each([ + ['group-[]:flex', 'group-[&]:flex'], + ['group-[]/name:flex', 'group-[&]/name:flex'], + + ['peer-[]:flex', 'peer-[&]:flex'], + ['peer-[]/name:flex', 'peer-[&]/name:flex'], +])('%s => %s (%#)', async (candidate, result) => { + let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', { + base: __dirname, + }) + + expect(handleEmptyArbitraryValues(designSystem, {}, candidate)).toEqual(result) +}) + +test.each([ + ['group-[]:tw-flex', 'tw:group-[&]:flex'], + ['group-[]/name:tw-flex', 'tw:group-[&]/name:flex'], + + ['peer-[]:tw-flex', 'tw:peer-[&]:flex'], + ['peer-[]/name:tw-flex', 'tw:peer-[&]/name:flex'], +])('%s => %s (%#)', async (candidate, result) => { + let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss" prefix(tw);', { + base: __dirname, + }) + + expect( + [handleEmptyArbitraryValues, prefix].reduce( + (acc, step) => step(designSystem, { prefix: 'tw-' }, acc), + candidate, + ), + ).toEqual(result) +}) diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.ts b/packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.ts new file mode 100644 index 000000000000..8f7dc5e6eea2 --- /dev/null +++ b/packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.ts @@ -0,0 +1,27 @@ +import type { Config } from '../../../../tailwindcss/src/compat/plugin-api' +import type { DesignSystem } from '../../../../tailwindcss/src/design-system' + +export function handleEmptyArbitraryValues( + designSystem: DesignSystem, + _userConfig: Config, + rawCandidate: string, +): string { + // We can parse the candidate, nothing to do + if (designSystem.parseCandidate(rawCandidate).length > 0) { + return rawCandidate + } + + // No need to handle empty arbitrary values + if (!rawCandidate.includes('[]')) { + return rawCandidate + } + + // Add the `&` placeholder to the empty arbitrary values. Other codemods might + // migrate these away, but if not, then it's at least valid to parse. + // + // E.g.: `group-[]:flex` => `group-[&]:flex` + // E.g.: `group-[]/name:flex` => `group-[&]/name:flex` + return rawCandidate + .replaceAll('-[]:', '-[&]:') // End of variant + .replaceAll('-[]/', '-[&]/') // With modifier +} diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.test.ts index 442971ed0d30..ec38d08641cb 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.test.ts @@ -1,5 +1,6 @@ import { __unstable__loadDesignSystem } from '@tailwindcss/node' import { expect, test } from 'vitest' +import { handleEmptyArbitraryValues } from './handle-empty-arbitrary-values' import { modernizeArbitraryValues } from './modernize-arbitrary-values' import { prefix } from './prefix' @@ -31,6 +32,10 @@ test.each([ ['group-[]:flex', 'in-[.group]:flex'], ['group-[]/name:flex', 'in-[.group\\/name]:flex'], + // Migrate `peer-[]` to a parsable `peer-[&]` instead: + ['peer-[]:flex', 'peer-[&]:flex'], + ['peer-[]/name:flex', 'peer-[&]/name:flex'], + // These shouldn't happen in the real world (because compound variants are // new). But this could happen once we allow codemods to run in v4+ projects. ['has-group-[]:flex', 'has-in-[.group]:flex'], @@ -85,12 +90,17 @@ test.each([ ['has-[[aria-visible]]:flex', 'has-aria-[visible]:flex'], ['has-[&:not(:nth-child(even))]:flex', 'has-odd:flex'], -])('%s => %s', async (candidate, result) => { +])('%s => %s (%#)', async (candidate, result) => { let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', { base: __dirname, }) - expect(modernizeArbitraryValues(designSystem, {}, candidate)).toEqual(result) + expect( + [handleEmptyArbitraryValues, modernizeArbitraryValues].reduce( + (acc, step) => step(designSystem, {}, acc), + candidate, + ), + ).toEqual(result) }) test.each([ @@ -101,6 +111,10 @@ test.each([ ['group-[]:tw-flex', 'tw:in-[.tw\\:group]:flex'], ['group-[]/name:tw-flex', 'tw:in-[.tw\\:group\\/name]:flex'], + // Migrate `peer-[]` to a parsable `peer-[&]` instead: + ['peer-[]:tw-flex', 'tw:peer-[&]:flex'], + ['peer-[]/name:tw-flex', 'tw:peer-[&]/name:flex'], + // However, `.group` inside of an arbitrary variant should not be prefixed: ['[.group_&]:tw-flex', 'tw:in-[.group]:flex'], @@ -110,13 +124,13 @@ test.each([ ['has-group-[]/name:tw-flex', 'tw:has-in-[.tw\\:group\\/name]:flex'], ['not-group-[]:tw-flex', 'tw:not-in-[.tw\\:group]:flex'], ['not-group-[]/name:tw-flex', 'tw:not-in-[.tw\\:group\\/name]:flex'], -])('%s => %s', async (candidate, result) => { +])('%s => %s (%#)', async (candidate, result) => { let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss" prefix(tw);', { base: __dirname, }) expect( - [prefix, modernizeArbitraryValues].reduce( + [handleEmptyArbitraryValues, prefix, modernizeArbitraryValues].reduce( (acc, step) => step(designSystem, { prefix: 'tw-' }, acc), candidate, ), diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.ts b/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.ts index 215b4533f79b..3d90351b1433 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.ts @@ -43,7 +43,7 @@ export function modernizeArbitraryValues( variant.kind === 'compound' && variant.root === 'group' && variant.variant.kind === 'arbitrary' && - variant.variant.selector === '&:is()' + variant.variant.selector === '&' ) { // `group-[]` if (variant.modifier === null) { diff --git a/packages/@tailwindcss-upgrade/src/template/migrate.ts b/packages/@tailwindcss-upgrade/src/template/migrate.ts index dd043c34b2e9..7fdc164f23d8 100644 --- a/packages/@tailwindcss-upgrade/src/template/migrate.ts +++ b/packages/@tailwindcss-upgrade/src/template/migrate.ts @@ -6,6 +6,7 @@ import { extractRawCandidates } from './candidates' import { arbitraryValueToBareValue } from './codemods/arbitrary-value-to-bare-value' import { automaticVarInjection } from './codemods/automatic-var-injection' import { bgGradient } from './codemods/bg-gradient' +import { handleEmptyArbitraryValues } from './codemods/handle-empty-arbitrary-values' import { important } from './codemods/important' import { legacyArbitraryValues } from './codemods/legacy-arbitrary-values' import { legacyClasses } from './codemods/legacy-classes' @@ -29,6 +30,7 @@ export type Migration = ( ) => string | Promise export const DEFAULT_MIGRATIONS: Migration[] = [ + handleEmptyArbitraryValues, prefix, important, bgGradient, diff --git a/packages/tailwindcss/src/candidate.test.ts b/packages/tailwindcss/src/candidate.test.ts index d7f458061715..4242b11b7164 100644 --- a/packages/tailwindcss/src/candidate.test.ts +++ b/packages/tailwindcss/src/candidate.test.ts @@ -1416,3 +1416,130 @@ it('should parse candidates with a prefix', () => { ] `) }) + +it.each([ + // Empty arbitrary value + 'bg-[]', + 'bg-()', + // — Tricking the parser with a space is not allowed + 'bg-[_]', + 'bg-(_)', + + // Empty arbitrary value, with typehint + 'bg-[color:]', + 'bg-(color:)', + // — Tricking the parser with a space is not allowed + 'bg-[color:_]', + 'bg-(color:_)', + + // Empty arbitrary modifier + 'bg-red-500/[]', + 'bg-red-500/()', + // — Tricking the parser with a space is not allowed + 'bg-red-500/[_]', + 'bg-red-500/(_)', + + // Empty arbitrary modifier for arbitrary properties + '[color:red]/[]', + '[color:red]/()', + // — Tricking the parser with a space is not allowed + '[color:red]/[_]', + '[color:red]/(_)', + + // Empty arbitrary value and modifier + 'bg-[]/[]', + 'bg-()/[]', + 'bg-[]/()', + 'bg-()/()', + // — Tricking the parser with a space is not allowed + 'bg-[_]/[]', + 'bg-(_)/[]', + 'bg-[_]/()', + 'bg-(_)/()', + 'bg-[]/[_]', + 'bg-()/[_]', + 'bg-[]/(_)', + 'bg-()/(_)', + 'bg-[_]/[_]', + 'bg-(_)/[_]', + 'bg-[_]/(_)', + 'bg-(_)/(_)', + + // Functional variants + // Empty arbitrary value in variant + 'data-[]:flex', + 'data-():flex', + // — Tricking the parser with a space is not allowed + 'data-[_]:flex', + 'data-(_):flex', + + // Empty arbitrary modifier in variant + 'data-foo/[]:flex', + 'data-foo/():flex', + // — Tricking the parser with a space is not allowed + 'data-foo/[_]:flex', + 'data-foo/(_):flex', + + // Empty arbitrary value and modifier in variant + 'data-[]/[]:flex', + 'data-()/[]:flex', + 'data-[]/():flex', + 'data-()/():flex', + // — Tricking the parser with a space is not allowed + 'data-[_]/[]:flex', + 'data-(_)/[]:flex', + 'data-[_]/():flex', + 'data-(_)/():flex', + 'data-[]/[_]:flex', + 'data-()/[_]:flex', + 'data-[]/(_):flex', + 'data-()/(_):flex', + 'data-[_]/[_]:flex', + 'data-(_)/[_]:flex', + 'data-[_]/(_):flex', + 'data-(_)/(_):flex', + + // Compound variants + // Empty arbitrary value in variant + 'group-data-[]:flex', + 'group-data-():flex', + // — Tricking the parser with a space is not allowed + 'group-data-[_]:flex', + 'group-data-(_):flex', + + // Empty arbitrary modifier in variant + 'group-data-foo/[]:flex', + 'group-data-foo/():flex', + // — Tricking the parser with a space is not allowed + 'group-data-foo/[_]:flex', + 'group-data-foo/(_):flex', + + // Empty arbitrary value and modifier in variant + 'group-data-[]/[]:flex', + 'group-data-()/[]:flex', + 'group-data-[]/():flex', + 'group-data-()/():flex', + // — Tricking the parser with a space is not allowed + 'group-data-[_]/[]:flex', + 'group-data-(_)/[]:flex', + 'group-data-[_]/():flex', + 'group-data-(_)/():flex', + 'group-data-[]/[_]:flex', + 'group-data-()/[_]:flex', + 'group-data-[]/(_):flex', + 'group-data-()/(_):flex', + 'group-data-[_]/[_]:flex', + 'group-data-(_)/[_]:flex', + 'group-data-[_]/(_):flex', + 'group-data-(_)/(_):flex', +])('should not parse invalid empty arbitrary values: %s', (rawCandidate) => { + let utilities = new Utilities() + utilities.static('flex', () => []) + utilities.functional('bg', () => []) + + let variants = new Variants() + variants.functional('data', () => {}) + variants.compound('group', Compounds.StyleRules, () => {}) + + expect(run(rawCandidate, { utilities, variants })).toEqual([]) +}) diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index c5c8bd85c679..26def1bff0ee 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -288,6 +288,14 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter // - `bg-red-500/50/50` if (additionalModifier) return + let parsedModifier = modifierSegment === null ? null : parseModifier(modifierSegment) + + // Empty arbitrary values are invalid. E.g.: `[color:red]/[]` or `[color:red]/()`. + // ^^ ^^ + // `bg-[#0088cc]/[]` or `bg-[#0088cc]/()`. + // ^^ ^^ + if (modifierSegment !== null && parsedModifier === null) return + // Arbitrary properties if (baseWithoutModifier[0] === '[') { // Arbitrary properties should end with a `]`. @@ -322,7 +330,7 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter kind: 'arbitrary', property, value, - modifier: modifierSegment === null ? null : parseModifier(modifierSegment), + modifier: parsedModifier, variants: parsedCandidateVariants, important, raw: input, @@ -413,7 +421,7 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter let candidate: Candidate = { kind: 'functional', root, - modifier: modifierSegment === null ? null : parseModifier(modifierSegment), + modifier: parsedModifier, value: null, variants: parsedCandidateVariants, important, @@ -430,7 +438,7 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter let valueIsArbitrary = startArbitraryIdx !== -1 if (valueIsArbitrary) { - let arbitraryValue = value.slice(startArbitraryIdx + 1, -1) + let arbitraryValue = decodeArbitraryValue(value.slice(startArbitraryIdx + 1, -1)) // Extract an explicit typehint if present, e.g. `bg-[color:var(--my-var)])` let typehint = '' @@ -453,10 +461,16 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter break } + // Empty arbitrary values are invalid. E.g.: `p-[]` + // ^^ + if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) { + continue + } + candidate.value = { kind: 'arbitrary', dataType: typehint || null, - value: decodeArbitraryValue(arbitraryValue), + value: arbitraryValue, } } else { // Some utilities support fractions as values, e.g. `w-1/2`. Since it's @@ -479,22 +493,30 @@ export function* parseCandidate(input: string, designSystem: DesignSystem): Iter } } -function parseModifier(modifier: string): CandidateModifier { +function parseModifier(modifier: string): CandidateModifier | null { if (modifier[0] === '[' && modifier[modifier.length - 1] === ']') { - let arbitraryValue = modifier.slice(1, -1) + let arbitraryValue = decodeArbitraryValue(modifier.slice(1, -1)) + + // Empty arbitrary values are invalid. E.g.: `data-[]:` + // ^^ + if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null return { kind: 'arbitrary', - value: decodeArbitraryValue(arbitraryValue), + value: arbitraryValue, } } if (modifier[0] === '(' && modifier[modifier.length - 1] === ')') { - let arbitraryValue = modifier.slice(1, -1) + let arbitraryValue = decodeArbitraryValue(modifier.slice(1, -1)) + + // Empty arbitrary values are invalid. E.g.: `data-():` + // ^^ + if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null return { kind: 'arbitrary', - value: decodeArbitraryValue(`var(${arbitraryValue})`), + value: `var(${arbitraryValue})`, } } @@ -524,6 +546,10 @@ export function parseVariant(variant: string, designSystem: DesignSystem): Varia let selector = decodeArbitraryValue(variant.slice(1, -1)) + // Empty arbitrary values are invalid. E.g.: `[]:` + // ^^ + if (selector.length === 0 || selector.trim().length === 0) return null + let relative = selector[0] === '>' || selector[0] === '+' || selector[0] === '~' // Ensure `&` is always present by wrapping the selector in `&:is(…)`, @@ -577,35 +603,52 @@ export function parseVariant(variant: string, designSystem: DesignSystem): Varia } case 'functional': { + let parsedModifier = modifier === null ? null : parseModifier(modifier) + // Empty arbitrary values are invalid. E.g.: `@max-md/[]:` or `@max-md/():` + // ^^ ^^ + if (modifier !== null && parsedModifier === null) return null + if (value === null) { return { kind: 'functional', root, - modifier: modifier === null ? null : parseModifier(modifier), + modifier: parsedModifier, value: null, } } if (value[0] === '[' && value[value.length - 1] === ']') { + let arbitraryValue = decodeArbitraryValue(value.slice(1, -1)) + + // Empty arbitrary values are invalid. E.g.: `data-[]:` + // ^^ + if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null + return { kind: 'functional', root, - modifier: modifier === null ? null : parseModifier(modifier), + modifier: parsedModifier, value: { kind: 'arbitrary', - value: decodeArbitraryValue(value.slice(1, -1)), + value: arbitraryValue, }, } } if (value[0] === '(' && value[value.length - 1] === ')') { + let arbitraryValue = decodeArbitraryValue(value.slice(1, -1)) + + // Empty arbitrary values are invalid. E.g.: `data-():` + // ^^ + if (arbitraryValue.length === 0 || arbitraryValue.trim().length === 0) return null + return { kind: 'functional', root, - modifier: modifier === null ? null : parseModifier(modifier), + modifier: parsedModifier, value: { kind: 'arbitrary', - value: decodeArbitraryValue(`var(${value.slice(1, -1)})`), + value: `var(${arbitraryValue})`, }, } } @@ -613,7 +656,7 @@ export function parseVariant(variant: string, designSystem: DesignSystem): Varia return { kind: 'functional', root, - modifier: modifier === null ? null : parseModifier(modifier), + modifier: parsedModifier, value: { kind: 'named', value }, } } @@ -627,10 +670,15 @@ export function parseVariant(variant: string, designSystem: DesignSystem): Varia // These two variants must be compatible when compounded if (!designSystem.variants.compoundsWith(root, subVariant)) return null + let parsedModifier = modifier === null ? null : parseModifier(modifier) + // Empty arbitrary values are invalid. E.g.: `group-focus/[]:` or `group-focus/():` + // ^^ ^^ + if (modifier !== null && parsedModifier === null) return null + return { kind: 'compound', root, - modifier: modifier === null ? null : { kind: 'named', value: modifier }, + modifier: parsedModifier, variant: subVariant, } } diff --git a/packages/tailwindcss/src/variants.test.ts b/packages/tailwindcss/src/variants.test.ts index c6510fc26fdc..aa2c857956bc 100644 --- a/packages/tailwindcss/src/variants.test.ts +++ b/packages/tailwindcss/src/variants.test.ts @@ -517,7 +517,7 @@ test('group-[...]', async () => { css` @tailwind utilities; `, - ['group-[@media_foo]:flex', 'group-[>img]:flex'], + ['group-[]:flex', 'group-hover/[]:flex', 'group-[@media_foo]:flex', 'group-[>img]:flex'], ), ).toEqual('') }) @@ -609,7 +609,7 @@ test('peer-[...]', async () => { css` @tailwind utilities; `, - ['peer-[@media_foo]:flex', 'peer-[>img]:flex'], + ['peer-[]:flex', 'peer-hover/[]:flex', 'peer-[@media_foo]:flex', 'peer-[>img]:flex'], ), ).toEqual('') }) @@ -1853,6 +1853,7 @@ test('data', async () => { `) expect( await run([ + 'data-[]:flex', 'data-[foo_^_=_"bar"]:flex', // Can't have spaces between `^` and `=` 'data-disabled/foo:flex', 'data-[potato=salad]/foo:flex',