Skip to content

Commit

Permalink
Don't output unparsable arbitrary values (#7789)
Browse files Browse the repository at this point in the history
* Refactor

* Don’t output unparsable arbitrary values

* Update changelog
  • Loading branch information
thecrypticace committed Mar 8, 2022
1 parent c6097d5 commit 68d896b
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Only add `!` to selector class matching template candidate when using important modifier with mutli-class selectors ([#7664](https://github.com/tailwindlabs/tailwindcss/pull/7664))
- Correctly parse and prefix animation names with dots ([#7163](https://github.com/tailwindlabs/tailwindcss/pull/7163))
- Fix extraction from template literal/function with array ([#7481](https://github.com/tailwindlabs/tailwindcss/pull/7481))
- Don't output unparsable arbitrary values ([#7789](https://github.com/tailwindlabs/tailwindcss/pull/7789))

### Changed

Expand Down
105 changes: 62 additions & 43 deletions src/lib/generateRules.js
Expand Up @@ -303,6 +303,19 @@ function looksLikeUri(declaration) {
}
}

function isParsableNode(node) {
let isParsable = true

node.walkDecls((decl) => {
if (!isParsableCssValue(decl.name, decl.value)) {
isParsable = false
return false
}
})

return isParsable
}

function isParsableCssValue(property, value) {
// We don't want to to treat [https://example.com] as a custom property
// Even though, according to the CSS grammar, it's a totally valid CSS declaration
Expand Down Expand Up @@ -456,60 +469,66 @@ function* resolveMatches(candidate, context) {
}
}

// Only keep the result of the very first plugin if we are dealing with
// arbitrary values, to protect against ambiguity.
if (isArbitraryValue(modifier) && matches.length > 1) {
let typesPerPlugin = matches.map((match) => new Set([...(typesByMatches.get(match) ?? [])]))
if (isArbitraryValue(modifier)) {
// When generated arbitrary values are ambiguous, we can't know
// which to pick so don't generate any utilities for them
if (matches.length > 1) {
let typesPerPlugin = matches.map((match) => new Set([...(typesByMatches.get(match) ?? [])]))

// Remove duplicates, so that we can detect proper unique types for each plugin.
for (let pluginTypes of typesPerPlugin) {
for (let type of pluginTypes) {
let removeFromOwnGroup = false
// Remove duplicates, so that we can detect proper unique types for each plugin.
for (let pluginTypes of typesPerPlugin) {
for (let type of pluginTypes) {
let removeFromOwnGroup = false

for (let otherGroup of typesPerPlugin) {
if (pluginTypes === otherGroup) continue
for (let otherGroup of typesPerPlugin) {
if (pluginTypes === otherGroup) continue

if (otherGroup.has(type)) {
otherGroup.delete(type)
removeFromOwnGroup = true
if (otherGroup.has(type)) {
otherGroup.delete(type)
removeFromOwnGroup = true
}
}
}

if (removeFromOwnGroup) pluginTypes.delete(type)
if (removeFromOwnGroup) pluginTypes.delete(type)
}
}
}

let messages = []

for (let [idx, group] of typesPerPlugin.entries()) {
for (let type of group) {
let rules = matches[idx]
.map(([, rule]) => rule)
.flat()
.map((rule) =>
rule
.toString()
.split('\n')
.slice(1, -1) // Remove selector and closing '}'
.map((line) => line.trim())
.map((x) => ` ${x}`) // Re-indent
.join('\n')
let messages = []

for (let [idx, group] of typesPerPlugin.entries()) {
for (let type of group) {
let rules = matches[idx]
.map(([, rule]) => rule)
.flat()
.map((rule) =>
rule
.toString()
.split('\n')
.slice(1, -1) // Remove selector and closing '}'
.map((line) => line.trim())
.map((x) => ` ${x}`) // Re-indent
.join('\n')
)
.join('\n\n')

messages.push(
` Use \`${candidate.replace('[', `[${type}:`)}\` for \`${rules.trim()}\``
)
.join('\n\n')

messages.push(` Use \`${candidate.replace('[', `[${type}:`)}\` for \`${rules.trim()}\``)
break
break
}
}

log.warn([
`The class \`${candidate}\` is ambiguous and matches multiple utilities.`,
...messages,
`If this is content and not a class, replace it with \`${candidate
.replace('[', '[')
.replace(']', ']')}\` to silence this warning.`,
])
continue
}

log.warn([
`The class \`${candidate}\` is ambiguous and matches multiple utilities.`,
...messages,
`If this is content and not a class, replace it with \`${candidate
.replace('[', '[')
.replace(']', ']')}\` to silence this warning.`,
])
continue
matches = matches.map((list) => list.filter((match) => isParsableNode(match[1])))
}

matches = matches.flat()
Expand Down
14 changes: 14 additions & 0 deletions tests/arbitrary-values.test.js
Expand Up @@ -370,3 +370,17 @@ it('should be possible to read theme values in arbitrary values (with quotes) wh
`)
})
})

it('should not output unparsable arbitrary CSS values', () => {
let config = {
content: [
{
raw: 'let classes = `w-[${sizes.width}]`',
},
],
}

return run('@tailwind utilities', config).then((result) => {
return expect(result.css).toMatchFormattedCss(``)
})
})

0 comments on commit 68d896b

Please sign in to comment.