Skip to content

Commit

Permalink
Ensure @utility is processed before using them (#15542)
Browse files Browse the repository at this point in the history
This PR fixes an issue where using an `@utility` before it is defined,
and _if_ that `@utility` contains `@apply`, that it won't result in the
expected output. But results in an `@apply` rule that is not
substituted. Additionally, if you have multiple levels of `@apply`, we
have to make sure that everything is applied (no pun intended) in the
right order.

Right now, the following steps are taken:

1. Collect all the `@utility` at-rules (and register them in the system
as utilities).
2. Substitute `@apply` on the AST (including `@utility`'s ASTs) with the
content of the utility.
3. Delete the `@utility` at-rules such that they are removed from the
CSS output itself.

The reason we do it in this order is because handling `@apply` during
`@utility`
handling means that we could rely on another `@utility` that is defined
later
and therefore the order of the utilities starts to matter. This is not a
bad
thing, but the moment you import multiple CSS files or plugins, this
could
become hard to manage.

Another important step is that when using `@utility foo`, the
implementation creates a `structuredClone` from its AST when first using
the utility. The reason we do that is because `foo` and `foo!` generate
different output and we don't want to accidentally mutate the same AST.
This structured clone is the start of the problem in the linked issue
(#15501).

If we don't do the structured clone, then substituting the `@apply`
rules would work, but then `foo` and `foo!` will generate the same
output, which is bad.

The linked issue has this structure:
```css
.foo {
  @apply bar;
}

@Utility bar {
  @apply flex;
}
```

If we follow the steps above, this would substitute `@apply bar` first,
which
results in:

```css
.foo {
  @apply flex;
}
```

But the `bar` utility, was already cloned (and cached) so now we end up
with an `@apply` rule that is not substituted.

To properly solve this problem, we have to make sure that we collect all
the `@apply` at-rules, and apply them in the correct order. To do this,
we run a topological sort on them which ensures that all the
dependencies are applied before substituting the current `@apply`.

This means that in the above example, in order to process `@apply bar`,
we have to process the `bar` utility first.

If we run into a circular dependency, then we will throw an error like
before. You'll notice that the error message in this PR is updated to a
different spot. This one is a bit easier to grasp because it shows the
error where the circular dependency _starts_ not where it _ends_ (and
completes the circle). The previous message was not wrong (since it's a
circle), but now it's a bit easier to reason about.

Fixes: #15501
  • Loading branch information
RobinMalfait authored Jan 7, 2025
1 parent dcf116b commit 02cfc45
Show file tree
Hide file tree
Showing 5 changed files with 237 additions and 37 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix `inset-shadow-*` suggestions in IntelliSense ([#15471](https://github.com/tailwindlabs/tailwindcss/pull/15471))
- Only compile arbitrary values ending in `]` ([#15503](https://github.com/tailwindlabs/tailwindcss/pull/15503))
- Improve performance and memory usage ([#15529](https://github.com/tailwindlabs/tailwindcss/pull/15529))
- Ensure `@apply` rules are processed in the correct order ([#15542](https://github.com/tailwindlabs/tailwindcss/pull/15542))
- _Upgrade (experimental)_: Do not extract class names from functions (e.g. `shadow` in `filter: 'drop-shadow(…)'`) ([#15566](https://github.com/tailwindlabs/tailwindcss/pull/15566))

### Changed
Expand Down
189 changes: 157 additions & 32 deletions packages/tailwindcss/src/apply.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,29 @@
import { Features } from '.'
import { walk, WalkAction, type AstNode } from './ast'
import { rule, toCss, walk, WalkAction, type AstNode } from './ast'
import { compileCandidates } from './compile'
import type { DesignSystem } from './design-system'
import { escape } from './utils/escape'
import { DefaultMap } from './utils/default-map'

export function substituteAtApply(ast: AstNode[], designSystem: DesignSystem) {
let features = Features.None
walk(ast, (node, { replaceWith }) => {

// Wrap the whole AST in a root rule to make sure there is always a parent
// available for `@apply` at-rules. In some cases, the incoming `ast` just
// contains `@apply` at-rules which means that there is no proper parent to
// rely on.
let root = rule('&', ast)

// Track all nodes containing `@apply`
let parents = new Set<AstNode>()

// Track all the dependencies of an `AstNode`
let dependencies = new DefaultMap<AstNode, Set<string>>(() => new Set<string>())

// Track all `@utility` definitions by its root (name)
let definitions = new DefaultMap(() => new Set<AstNode>())

// Collect all new `@utility` definitions and all `@apply` rules first
walk([root], (node, { parent }) => {
if (node.kind !== 'at-rule') return

// Do not allow `@apply` rules inside `@keyframes` rules.
Expand All @@ -19,9 +36,119 @@ export function substituteAtApply(ast: AstNode[], designSystem: DesignSystem) {
return WalkAction.Skip
}

if (node.name !== '@apply') return
features |= Features.AtApply
// `@utility` defines a utility, which is important information in order to
// do a correct topological sort later on.
if (node.name === '@utility') {
let name = node.params.replace(/-\*$/, '')
definitions.get(name).add(node)

// In case `@apply` rules are used inside `@utility` rules.
walk(node.nodes, (child) => {
if (child.kind !== 'at-rule' || child.name !== '@apply') return

parents.add(node)

for (let dependency of resolveApplyDependencies(child, designSystem)) {
dependencies.get(node).add(dependency)
}
})
return
}

// Any other `@apply` node.
if (node.name === '@apply') {
// `@apply` cannot be top-level, so we need to have a parent such that we
// can replace the `@apply` node with the actual utility classes later.
if (parent === null) return

features |= Features.AtApply

parents.add(parent)

for (let dependency of resolveApplyDependencies(node, designSystem)) {
dependencies.get(parent).add(dependency)
}
}
})

// Topological sort before substituting `@apply`
let seen = new Set<AstNode>()
let sorted: AstNode[] = []
let wip = new Set<AstNode>()

function visit(node: AstNode, path: AstNode[] = []) {
if (seen.has(node)) {
return
}

// Circular dependency detected
if (wip.has(node)) {
// Next node in the path is the one that caused the circular dependency
let next = path[(path.indexOf(node) + 1) % path.length]

if (
node.kind === 'at-rule' &&
node.name === '@utility' &&
next.kind === 'at-rule' &&
next.name === '@utility'
) {
walk(node.nodes, (child) => {
if (child.kind !== 'at-rule' || child.name !== '@apply') return

let candidates = child.params.split(/\s+/g)
for (let candidate of candidates) {
for (let candidateAstNode of designSystem.parseCandidate(candidate)) {
switch (candidateAstNode.kind) {
case 'arbitrary':
break

case 'static':
case 'functional':
if (next.params.replace(/-\*$/, '') === candidateAstNode.root) {
throw new Error(
`You cannot \`@apply\` the \`${candidate}\` utility here because it creates a circular dependency.`,
)
}
break

default:
candidateAstNode satisfies never
}
}
}
})
}

// Generic fallback error in case we cannot properly detect the origin of
// the circular dependency.
throw new Error(
`Circular dependency detected:\n\n${toCss([node])}\nRelies on:\n\n${toCss([next])}`,
)
}

wip.add(node)

for (let dependencyId of dependencies.get(node)) {
for (let dependency of definitions.get(dependencyId)) {
path.push(node)
visit(dependency, path)
path.pop()
}
}

seen.add(node)
wip.delete(node)

sorted.push(node)
}

for (let node of parents) {
visit(node)
}

// Substitute the `@apply` at-rules in order
walk(sorted, (node, { replaceWith }) => {
if (node.kind !== 'at-rule' || node.name !== '@apply') return
let candidates = node.params.split(/\s+/g)

// Replace the `@apply` rule with the actual utility classes
Expand All @@ -48,35 +175,33 @@ export function substituteAtApply(ast: AstNode[], designSystem: DesignSystem) {
}
}

// Verify that we don't have any circular dependencies by verifying that
// the current node does not appear in the new nodes.
walk(newNodes, (child) => {
if (child !== node) return

// At this point we already know that we have a circular dependency.
//
// Figure out which candidate caused the circular dependency. This will
// help to create a useful error message for the end user.
for (let candidate of candidates) {
let selector = `.${escape(candidate)}`

for (let rule of candidateAst) {
if (rule.kind !== 'rule') continue
if (rule.selector !== selector) continue

walk(rule.nodes, (child) => {
if (child !== node) return

throw new Error(
`You cannot \`@apply\` the \`${candidate}\` utility here because it creates a circular dependency.`,
)
})
}
}
})

replaceWith(newNodes)
}
})

return features
}

function* resolveApplyDependencies(
node: Extract<AstNode, { kind: 'at-rule' }>,
designSystem: DesignSystem,
) {
for (let candidate of node.params.split(/\s+/g)) {
for (let node of designSystem.parseCandidate(candidate)) {
switch (node.kind) {
case 'arbitrary':
// Doesn't matter, because there is no lookup needed
break

case 'static':
case 'functional':
// Lookup by "root"
yield node.root
break

default:
node satisfies never
}
}
}
}
47 changes: 47 additions & 0 deletions packages/tailwindcss/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,53 @@ describe('@apply', () => {
}"
`)
})

it('should recursively apply with custom `@utility`, which is used before it is defined', async () => {
expect(
await compileCss(
css`
@tailwind utilities;
@layer base {
body {
@apply a;
}
}
@utility a {
@apply b;
}
@utility b {
@apply focus:c;
}
@utility c {
@apply my-flex!;
}
@utility my-flex {
@apply flex;
}
`,
['a', 'b', 'c', 'flex', 'my-flex'],
),
).toMatchInlineSnapshot(`
".a:focus, .b:focus, .c {
display: flex !important;
}
.flex, .my-flex {
display: flex;
}
@layer base {
body:focus {
display: flex !important;
}
}"
`)
})
})

describe('arbitrary variants', () => {
Expand Down
4 changes: 1 addition & 3 deletions packages/tailwindcss/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,8 @@ async function parseCss(
node.context = {}
}

// Replace `@apply` rules with the actual utility classes.
features |= substituteAtApply(ast, designSystem)

features |= substituteFunctions(ast, designSystem.resolveThemeValue)
features |= substituteAtApply(ast, designSystem)

// Remove `@utility`, we couldn't replace it before yet because we had to
// handle the nested `@apply` at-rules first.
Expand Down
33 changes: 31 additions & 2 deletions packages/tailwindcss/src/utilities.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17375,7 +17375,7 @@ describe('custom utilities', () => {
['foo', 'bar'],
),
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: You cannot \`@apply\` the \`dark:foo\` utility here because it creates a circular dependency.]`,
`[Error: You cannot \`@apply\` the \`hover:bar\` utility here because it creates a circular dependency.]`,
)
})

Expand Down Expand Up @@ -17406,7 +17406,36 @@ describe('custom utilities', () => {
['foo', 'bar'],
),
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: You cannot \`@apply\` the \`dark:foo\` utility here because it creates a circular dependency.]`,
`[Error: You cannot \`@apply\` the \`hover:bar\` utility here because it creates a circular dependency.]`,
)
})

test('custom utilities with `@apply` causing circular dependencies should error (multiple levels)', async () => {
await expect(() =>
compileCss(
css`
body {
@apply foo;
}
@utility foo {
@apply flex-wrap hover:bar;
}
@utility bar {
@apply flex dark:baz;
}
@utility baz {
@apply flex-wrap hover:foo;
}
@tailwind utilities;
`,
['foo', 'bar'],
),
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: You cannot \`@apply\` the \`hover:bar\` utility here because it creates a circular dependency.]`,
)
})
})

0 comments on commit 02cfc45

Please sign in to comment.