Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve custom properties in keyframes #16376

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Preserve custom properties in keyframes #16376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9bfa186
Add failing test
adamwathan Feb 8, 2025
dbc09f5
Fix
thecrypticace Feb 9, 2025
aba1f7d
Update changelog
thecrypticace Feb 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Nothing yet!
### Fixed

- Preserve custom properties in keyframes ([#16376](https://github.com/tailwindlabs/tailwindcss/pull/16376))

## [4.0.5] - 2025-02-08

Expand Down
8 changes: 7 additions & 1 deletion packages/tailwindcss/src/ast.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,9 @@ export function optimizeAst(ast: AstNode[], designSystem: DesignSystem) {

// Track variables defined in `@theme`
if (context.theme && node.property[0] === '-' && node.property[1] === '-') {
cssThemeVariables.get(parent).add(node)
if (!context.keyframes) {
cssThemeVariables.get(parent).add(node)
}
}

// Track used CSS variables
Expand Down Expand Up @@ -339,6 +341,10 @@ export function optimizeAst(ast: AstNode[], designSystem: DesignSystem) {

// AtRule
else if (node.kind === 'at-rule') {
if (node.name === '@keyframes') {
context = { ...context, keyframes: true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thecrypticace @RobinMalfait I was wondering if we can clean this up a bit by starting to use an AST walk instead and relying on the parentNode being set correctly to see wether we're inside a keyframe to avoid touching any properties. WDYT?

The code to remove nodes inside a context.reference boundary would become more annoying since we'll have to remove every sibling node instead of removing the whole context node (as context nodes are invisible for traversal).

Copy link
Member

@RobinMalfait RobinMalfait Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an option, but by the time we see a declaration, the parent node will never be the @keyframes node, but will be the intermediate rule:

@kefyrames {
  0% { /* <- this will be the parent */
    --foo: red;
  }
}

Or do you mean that at the the time we reach @keyframes, we do a walk in this block, and then mark all found declarations as "don't touch me"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was thinking of doing a return WalkAction.Skip maybe.

}

let copy = { ...node, nodes: [] }
for (let child of node.nodes) {
transform(child, copy.nodes, context, depth + 1)
Expand Down
36 changes: 36 additions & 0 deletions packages/tailwindcss/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,42 @@ describe('Parsing themes values from CSS', () => {
`)
})

// https://github.com/tailwindlabs/tailwindcss/issues/16374
test('custom properties in keyframes preserved', async () => {
expect(
await compileCss(
css`
@theme {
--animate-foo: used 1s infinite;

@keyframes used {
to {
--angle: 360deg;
}
}
}

@tailwind utilities;
`,
['animate-foo'],
),
).toMatchInlineSnapshot(`
":root, :host {
--animate-foo: used 1s infinite;
}

.animate-foo {
animation: var(--animate-foo);
}

@keyframes used {
to {
--angle: 360deg;
}
}"
`)
})

test('keyframes are generated when used in an animation using `@theme inline`', async () => {
expect(
await compileCss(
Expand Down