premailer / css_parser

Ruby CSS Parser
Other
279 stars 110 forks source link

Preserve rules priority while expanding shorthands #119

Closed ojab closed 3 years ago

ojab commented 3 years ago

We can piggyback on Hash preserving it's order in all supported rubies & drop order. Looks quite ugly, but does the trick. I assume that it will be faster if we'll properly track and propagate order in all places, but I suspect it will be also harder to work with.

And we're little faster at parsing because of order is removed: Before:

Executing: bundle exec rake benchmark
Warming up --------------------------------------
 import1.css loading   376.000  i/100ms
 complex.css loading    14.000  i/100ms
Calculating -------------------------------------
 import1.css loading      3.670k (± 3.6%) i/s -     18.424k in   5.027200s
 complex.css loading    142.421  (± 4.2%) i/s -    714.000  in   5.021913s

Loading `import1.css` allocated 372 objects, 26 KiB
Loading `complex.css` allocated 10437 objects, 752 KiB

After:

Executing: bundle exec rake benchmark
Warming up --------------------------------------
 import1.css loading   394.000  i/100ms
 complex.css loading    14.000  i/100ms
Calculating -------------------------------------
 import1.css loading      3.809k (± 3.7%) i/s -     19.306k in   5.075438s
 complex.css loading    147.464  (± 3.4%) i/s -    742.000  in   5.037139s

Loading `import1.css` allocated 357 objects, 25 KiB
Loading `complex.css` allocated 9536 objects, 680 KiB

And little slower at expanding shorthands (tested on github's 500KB css with parser.each_rule_set { |rule_set, _media_type| rule_set.expand_shorthand! }, not sure how relevant this file is though) Before:

Executing: bundle exec ruby b.rb
Warming up --------------------------------------
github-bf18d76d7dfd1ad6ed77f31817d6c749.css loading
                         1.000  i/100ms
Calculating -------------------------------------
github-bf18d76d7dfd1ad6ed77f31817d6c749.css loading
                          2.798  (± 0.0%) i/s -     84.000  in  30.089636s
Loading `github.css` and expanding shothands allocated 439154 objects, 29732 KiB

After:

Executing: bundle exec ruby b.rb
Warming up --------------------------------------
github-bf18d76d7dfd1ad6ed77f31817d6c749.css loading
                         1.000  i/100ms
Calculating -------------------------------------
github-bf18d76d7dfd1ad6ed77f31817d6c749.css loading
                          2.789  (± 0.0%) i/s -     84.000  in  30.161549s
Loading `github.css` and expanding shothands allocated 437074 objects, 30186 KiB
Successfully rebased and updated refs/heads/fixup_expanding_shorthands.

Fixes #111

ojab commented 3 years ago

So yeah, it's more or less RFC at this point and I will add Declarations#replace_declaration! tests, just want to make sure that this approach is acceptable.

grosser commented 3 years ago

CI failed

ojab commented 3 years ago

Yeah, trying to test forwardable, but looks like it's not trivial here https://github.com/seattlerb/minitest/issues/545

ojab commented 3 years ago

Comments are more or less addressed, CI is passing, I think it's ready if everything is fine for you.

ojab commented 3 years ago

Thanks :tada:! Can I haz release?

grosser commented 3 years ago

1.8.0

On Thu, Jan 21, 2021 at 11:36 AM Slava Kardakov notifications@github.com wrote:

Thanks 🎉! Can I haz release?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/premailer/css_parser/pull/119#issuecomment-764887413, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACYZ372C227ZMGZHBL4F3S3B62PANCNFSM4WJEACNA .