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

perf: minify css only when needed #5178

Merged
merged 4 commits into from Oct 23, 2021
Merged

Conversation

patak-dev
Copy link
Member

Description

Avoid minifying CSS if it isn't going to be used.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Could you explain why this is better?
Currently I read that there is a css variable in line 313
This is also used in line 322/320

Now that the else statement is not there anymore, the css variable gets minified also if line 313 gets executed.
Was this intended?

@patak-dev
Copy link
Member Author

Good catch @Shinigami92, corrected the code to avoid minification when not inlined

Shinigami92
Shinigami92 previously approved these changes Oct 2, 2021
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Yeah, now it makes sense 👍

@LifeIsStrange
Copy link

I had read somewhere that minifying allow for faster parsing so if this is true then if this css file will be parsed for e.g if a css class will be used then the analysis could be faster if minified.
However I don't know the topic nor do I even know if the unused check has granularity over css classes/selectors or is per file.
So my comment probably is irrelevant and wrong but I'm still saying it in case someone more knowledgeable thinks this is relevant

@patak-dev patak-dev requested a review from antfu October 14, 2021 11:12
@patak-dev patak-dev added the p2-nice-to-have Not breaking anything but nice to have (priority) label Oct 14, 2021
@patak-dev patak-dev merged commit 7970239 into main Oct 23, 2021
@patak-dev patak-dev deleted the perf-minify-css-when-needed branch October 23, 2021 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants