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

Jump to bottom

Replace postcss-csso as minifier #23

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Replace postcss-csso as minifier #23

merged 2 commits into from
Oct 2, 2024

Conversation

brookback
Copy link
Contributor

@brookback brookback commented Aug 23, 2024 •
edited
Loading

It's problematic, since it doesn't recognise at-rules such as @container and @starting-style. It's also quite unmaintained.

joakim-lookback
joakim-lookback approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@joakim-lookback joakim-lookback left a comment

Choose a reason for hiding this comment

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

Looks good.

Did you compare the bundle sizes with just minify instead of csso?

@brookback
Copy link
Contributor Author

Hm, 116K for new module vs. 128K with CSSO. Didn't think there would be that big of a difference.

@joakim-lookback
Copy link
Contributor

I thought CSSO should be smaller given how they talk about "structural optimizations" etc. Glad that it wasn't the case. Less config and smaller bundle sizes. Nice!

@brookback
Copy link
Contributor Author

@joakim-lookback SORRY, it was the other way around :D

CSSO

→ npm ls postcss-csso
[email protected] /Users/brookie/code/dashboard
└─┬ @lookback/[email protected]
  └── [email protected] (git+ssh://[email protected]/lookback/postcss-csso.git#6cb7497ebb1c3788e2304267ac30b244f7809947)
→ ENV=production BUNDLE=1 npm run build:css
Finished src/css/index.css in 941 ms
→ s build/bundle.css
116K	build/bundle.css
116K	total

New

→ npm ls @csstools/postcss-minify
[email protected] /Users/brookie/code/dashboard
└─┬ @lookback/[email protected] -> ./../lookbook
  └── @csstools/[email protected]
→ ENV=production BUNDLE=1 npm run build:css
Finished src/css/index.css in 963 ms
→ s build/bundle.css
128K	build/bundle.css
128K	total

@brookback
Copy link
Contributor Author

PostCSS Minify clearly states it won't do transformations: https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-minify

Compared to other minifiers, PostCSS Minify is purely focused on correctness and fidelity.
PostCSS Minify only collapses whitespace and comments while preserving those comments that are important for legal compliance.

PostCSS Minify is not a CSS optimizer, it does not try to reduce the size of the CSS file by altering the CSS itself.

@joakim-lookback
Copy link
Contributor

Right. I wonder if we should look into cssnano. It also fits right in into the postcss ecosystem and see if that will get us our bytes back. But then again, 10k, not much. It sure feels good to have a tool that is "focused on correctness and fidelity." 🙂

https://github.com/cssnano/cssnano

@brookback
Copy link
Contributor Author

@joakim-lookback We used cssnano before but switched to CSSO :) I think the reason was breakage in cssnano too.

@joakim-lookback
Copy link
Contributor

Aha! Had no idea. But cssnano looks maintained where csso does not. Perhaps it works for our usecase now?

@lolgesten
Copy link
Contributor

How does this affect the minifiers in the projects?

@brookback
Copy link
Contributor Author

@joakim-lookback I checked out cssnano again, and it's a config hellhole... Loading presets and whatnot which explodes in dependencies. I just want a CSS minifier to be drop-in.

Heard good things about http://lightningcss.dev.

@lolgesten This is the central minifier for the CSS in all microsites. When we do BUNDLE=1 they use this minifier exposed by the Lookbook.

@brookback
Copy link
Contributor Author

Bundle in dashboard becomes 112K with cssnano with the "default" preset. But it does heavy transformations.

@joakim-lookback
Copy link
Contributor

Who doesn't like a config hellhole? Is that worth 16k? Idk, perhaps not? Interesting dilemma. Dependencies vs asset weight here. Will be interesting to see where @lolgesten lands on this one 😄

@brookback
Copy link
Contributor Author

If using the default preset in cssnano, the deps aren't so bad. Since we only use it when bundling, it maybe is alright with heavy transformations. I mean, Typescript transpiles our TS source into unrecognizability in production anyway :)

@brookback
Copy link
Contributor Author

brookback commented Aug 23, 2024 •
edited
Loading

Minifying the main lookbook.css file:

main branch (csso) cssnano postcss-minify
240K 248K 264K

@brookback
Copy link
Contributor Author

brookback commented Aug 23, 2024 •
edited
Loading

Dependency trees

tldr; cssnano is 💀

cssnano 
└─ [email protected]
   ├─ [email protected]
   └─ [email protected]
      ├─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      │     ├─ [email protected]
      │     └─ [email protected]
      ├─ [email protected]
      ├─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  │  ├─ [email protected]
      │  │  └─ [email protected]
      │  ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  ├─ [email protected]
      │  │  ├─ [email protected]
      │  │  ├─ [email protected]
      │  │  ├─ [email protected]
      │  │  └─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      │     ├─ [email protected]
      │     └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  └─ [email protected]
      ├─ [email protected]
      │  ├─ [email protected]
      │  └─ [email protected]
      │     ├─ [email protected]
      │     │  ├─ [email protected]
      │     │  └─ [email protected]
      │     ├─ [email protected]
      │     │  ├─ [email protected]
      │     │  ├─ [email protected]
      │     │  ├─ [email protected]
      │     │  │  └─ [email protected]
      │     │  ├─ [email protected]
      │     │  │  ├─ [email protected]
      │     │  │  ├─ [email protected]
      │     │  │  │  ├─ [email protected]
      │     │  │  │  ├─ [email protected]
      │     │  │  │  └─ [email protected]
      │     │  │  └─ [email protected]
      │     │  └─ [email protected]
      │     │     └─ [email protected]
      │     ├─ [email protected]
      │     ├─ [email protected]
      │     ├─ [email protected]
      │     │  └─ [email protected]
      │     │     ├─ [email protected]
      │     │     └─ [email protected]
      │     ├─ @trysound/[email protected]
      │     └─ [email protected]
      └─ [email protected]
         └─ [email protected]
postcss-minify 
└─ [email protected]
   ├─ [email protected]
   └─ [email protected]
      ├─ [email protected]
      └─ [email protected]
postcss-csso 
└─ [email protected]
   └─ [email protected]
      └─ [email protected]
         ├─ [email protected]
         └─ [email protected]

@brookback
Copy link
Contributor Author

"LightningCSS doesn't use dependencies, we use RUST" 🔥

└─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   ├─ [email protected]
   └─ [email protected]

@brookback
Copy link
Contributor Author

Ok, returning to this, I think we should go with the original minifier in this PR, postcss-minify.

I used it on the dashboard bundle.css, and it yielded the same bundle size as CSSO after purging (104 kB).

@brookback brookback merged commit cbce903 into main Oct 2, 2024
1 check passed
@brookback brookback deleted the fix/replace-csso branch October 2, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joakim-lookback joakim-lookback joakim-lookback approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brookback @joakim-lookback @lolgesten