css / csso

CSS minifier with structural optimizations
https://css.github.io/csso/csso.html
MIT License
3.74k stars 189 forks source link

Removes required "to" and "from" keyframe in animation #448

Closed SethFalco closed 1 year ago

SethFalco commented 2 years ago

When an SVG has key frames with to or from, CSSO will remove them, which changes the animation.

Expected Behavior

The to and from frames should be preserved/converted, otherwise it modifies the animation.

For example: 0%,to{clip-path:inset(84%0 0)}, should not become 0%{clip-path:inset(84%0 0)}.

Minimal Working Example

The SVG I'm optimizing in SVGO.

<svg width="48" height="48" viewBox="0 0 12.7 12.7" xmlns="http://www.w3.org/2000/svg"><style>@keyframes a{0%,to{clip-path:inset(84%0 0)}4%,96%{clip-path:inset(74%0 0)}16%,52%,84%{clip-path:inset(16%0 0)}20%,48%{clip-path:inset(19%0 0)}24%,44%{clip-path:inset(26%0 0)}28%,64%,72%{clip-path:inset(29%0 0)}32%{clip-path:inset(45%0 0)}36%,40%{clip-path:inset(35%0 0)}60%,76%{clip-path:inset(23%0 0)}68%{clip-path:inset(32%0 0)}}@keyframes b{0%,32%,40%,68%,to{clip-path:inset(13%0 0)}4%{clip-path:inset(20%0 0)}76%,8%{clip-path:inset(23%0 0)}16%,84%{clip-path:inset(29%0 0)}28%,72%,96%{clip-path:inset(19%0 0)}36%{clip-path:inset(16%0 0)}48%{clip-path:inset(45%0 0)}52%{clip-path:inset(58%0 0)}56%{clip-path:inset(48%0 0)}60%{clip-path:inset(35%0 0)}64%,92%{clip-path:inset(26%0 0)}88%{clip-path:inset(39%0 0)}}@keyframes c{0%,to{clip-path:inset(52%0 0)}16%{clip-path:inset(0 0 0)}20%,84%{clip-path:inset(10%0 0)}24%,52%{clip-path:inset(26%0 0)}28%{clip-path:inset(39%0 0)}32%,44%{clip-path:inset(48%0 0)}36%{clip-path:inset(55%0 0)}40%{clip-path:inset(68%0 0)}48%,92%{clip-path:inset(32%0 0)}56%{clip-path:inset(16%0 0)}60%,76%,88%{clip-path:inset(23%0 0)}68%{clip-path:inset(29%0 0)}72%{clip-path:inset(35%0 0)}80%{clip-path:inset(13%0 0)}}</style><path d="M2.117 2.249h2.38v8.202h-2.38z" fill="#FFF" style="animation:a 1.25s linear infinite"/><path d="M5.292 2.249h2.38v8.202h-2.38z" fill="#FFF" style="animation:b 1.25s linear infinite"/><path d="M8.467 2.249h2.38v8.202h-2.38z" fill="#FFF" style="animation:c 1.25s linear infinite"/></svg>

A minimal example (using my actual use-case) of how it's being passed to CSSO.

const { minify } = require('csso');
const fs = require('fs');

const src = '@keyframes a{0%,to{clip-path:inset(84%0 0)}4%,96%{clip-path:inset(74%0 0)}16%,52%,84%{clip-path:inset(16%0 0)}20%,48%{clip-path:inset(19%0 0)}24%,44%{clip-path:inset(26%0 0)}28%,64%,72%{clip-path:inset(29%0 0)}32%{clip-path:inset(45%0 0)}36%,40%{clip-path:inset(35%0 0)}60%,76%{clip-path:inset(23%0 0)}68%{clip-path:inset(32%0 0)}}@keyframes b{0%,32%,40%,68%,to{clip-path:inset(13%0 0)}4%{clip-path:inset(20%0 0)}76%,8%{clip-path:inset(23%0 0)}16%,84%{clip-path:inset(29%0 0)}28%,72%,96%{clip-path:inset(19%0 0)}36%{clip-path:inset(16%0 0)}48%{clip-path:inset(45%0 0)}52%{clip-path:inset(58%0 0)}56%{clip-path:inset(48%0 0)}60%{clip-path:inset(35%0 0)}64%,92%{clip-path:inset(26%0 0)}88%{clip-path:inset(39%0 0)}}@keyframes c{0%,to{clip-path:inset(52%0 0)}16%{clip-path:inset(0 0 0)}20%,84%{clip-path:inset(10%0 0)}24%,52%{clip-path:inset(26%0 0)}28%{clip-path:inset(39%0 0)}32%,44%{clip-path:inset(48%0 0)}36%{clip-path:inset(55%0 0)}40%{clip-path:inset(68%0 0)}48%,92%{clip-path:inset(32%0 0)}56%{clip-path:inset(16%0 0)}60%,76%,88%{clip-path:inset(23%0 0)}68%{clip-path:inset(29%0 0)}72%{clip-path:inset(35%0 0)}80%{clip-path:inset(13%0 0)}}';

const optimized = minify(src, {
  usage: {
    tags: ['svg', 'style', 'path']
  }
});

fs.writeFile('minified.css', optimized.css, 'utf8', () => {
  console.log('done');
});

I think the issue is caused by to/from being interpreted as a TypeSelector in the AST, and then being stripped out because it's not in the whitelisted array of tags.

"prelude": {
  "type": "SelectorList",
  "loc": null,
  "children": [
    {
      "type": "Selector",
      "loc": null,
      "children": [
        {
          "type": "Percentage",
          "loc": null,
          "value": "0"
        }
      ]
    },
    {
      "type": "Selector",
      "loc": null,
      "children": [
        {
          "type": "TypeSelector",
          "loc": null,
          "name": "to"
        }
      ]
    }
  ]
},

Before I do anything else, I'm wondering, is CSSTree correct to report that as a TypeSelector? 🤔

This might be a bug in CSSTree, since to and from aren't type selectors. (https://www.w3.org/TR/selectors-3/#type-selectors) If they had to be named, it'd probably fall under keyframe selectors, or KeyframeSelector. (https://www.w3.org/TR/css-animations-1/#typedef-keyframe-selector)

Related

lahmatiy commented 1 year ago

That's a pretty interesting bug. There is no a big deal that css-tree treated from/to as TypeSelector but CSSO should avoid applying usage filtering to @keyframe rules as well as for other special at-rules like @page (not the case for SVGO but still). I think to fix it we need add an additional check here that a rule don't belong to a special at-rule like @keyframe, @page etc.