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.

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

animation property using var() has the wrong name, keyframes are excluded and animation is broken #13208

Closed
kirbysayshi opened this issue Mar 11, 2024 · 1 comment
Assignees

Comments

@kirbysayshi
Copy link

What version of Tailwind CSS are you using?

v2.2.19 and v3.4.1

What build tool (or framework if it abstracts the build tool) are you using?

vite 5.1.1
postcss 8.4.35
@remix-run/react 2.8.1

What version of Node.js are you using?

v20.10.0

What browser are you using?

NA

What operating system are you using?

macOS

Reproduction URL

https://play.tailwindcss.com/AE5DqIZeTW

Both of these divs should be animating, but only the second is: the difference is that the animation-name is specified first, instead of last.

Describe your issue
Hello! We recently upgraded to v2.2 from v2.1 (yes, we're very behind... 😅) and encountered this issue that is also present in the latest release.

At least in v2.2.19 and v3.4.1 the animation-name property is being incorrectly parsed, resulting in the matching keyframes being excluded from the CSS output.

The problem seems to be parseAnimationValue, which is confusing a var(...) for the animation name. Then the JIT presumably excludes the "unused" keyframes.

It's straightforward to verify that that the output is incorrect (this is v2.2, but the same is true for v3):

node -p "require('tailwindcss/lib/util/parseAnimationValue').default('slide-x var(--slide-duration) linear 0s 1 forwards')"
image

(name is incorrect)

Note that a real browser / CSS parser has no trouble discerning which of the values is actually the name, as shown by this JSFiddle! https://jsfiddle.net/kirbysayshi/6nkpe8yw/

The workaround is to put the animation name last, but this goes against the guidelines from MDN (which I'm sure everyone's aware of, given some comments I see in parseAnimationValue tests!):

If a value in the animation shorthand can be parsed as a value for an animation property other than animation-name, then the value will be applied to that property first and not to animation-name. For this reason, the recommended practice is to specify a value for animation-name as the last value in a list of values when using the animation shorthand;

(emphasis mine) from : https://developer.mozilla.org/en-US/docs/Web/CSS/animation#description

Related:

@RobinMalfait RobinMalfait self-assigned this Mar 21, 2024
@RobinMalfait
Copy link
Contributor

Hey!

This is an unfortunate issue because there is no way right now where we can guarantee the correct values. Let's share some examples to explain myself:

If you have something like slide-x var(--slide-duration), we can put the slide-x in the name position, but we don't know what the value of var(--slide-duration) is. The browser doesn't have an issue with this, because it can resolve the actual values at runtime to know what type the value is. We don't know what the value is because we don't run at runtime.

One potential solution is that we parse everything first and handle all the var(--…)'s at the end. This would solve the issues where var(--slide-duration) slide-x thinks that var(--slide-duration) is the name and slide-x is unknown. Always processing the var(--slide-duration) at the end would solve this.

However, that brings us to the next issue with this approach. Both the duration and delay are numbers and the browser will pick the first value as the duration and the second number as the delay. This means that in your case var(--slide-duration) 0s should be parsed as:

  • var(--slide-duration) — this should be the duration
  • 0s — this should be the delay

However, if we process var(--…) at the end, then it would parse as:

  • 0s — this is the duration, but should be the delay
  • var(--slide-duration) — this is the delay, but should be the duration

So there is no safe way for us to "guess" what the variable represents, just because we don't have access to the value. One potential solution we could implement is by guessing the type based on the variable name. In this case var(--slide-duration) contains duration so it should be a duration. But this will break the moment your design system uses --slide-dur and --slide-del as shorthands (just to make something up here).

All that said, I don't think we can solve this in Tailwind itself.

I think the only reasonable thing to do here is to put the animation-name first when defining your custom animation value. The parseAnimationValue parses the full animation, but we are currently only ever interested in the actual name so that we can link it to the correct keyframes.

I wish we could solve this at the parse level, but unfortunately that's not the case because we don't know the type of the underlying value it represents. I hope this makes sense and I hope the workaround of always using the actual name first is good enough.

Hope this helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants