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

Improve production build performance for the case of many small non-tailwind stylesheets #4644

Merged
merged 2 commits into from Jul 1, 2021

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Jun 14, 2021

Hi there!
We decided to use tailwind as a base of our design system. Our codebase is legacy-ish Vue 2 SPA with pretty liberal use of <style> blocks in our components, built with webpack (this info will be important later). Tailwind worked great and was delightful to work with. That is, until we got to production build part.

Turns out, just adding tailwind to our postcss plugins list added a couple of minutes to our build times. Actual usage did not seem to matter: commenting out every @apply and @tailwind rule did not seem to change anything: build with tailwind plugin included still was couple of minutes slower than without it. Further investigation pointed out at purge option as the culprit. Removing it seemed to bring out build times in line with what we expected, but we'd very much like to keep it. Of course, purging unused styles takes time, but minutes still seemed excessive, especially considering that tailwind-cli with the same config successfully generates the stylesheet in a couple of seconds.

As it turns out, tailwind plugin runs purgecss on every source stylesheet. Which, in case of legacy-ish code base with a lot of tiny stylesheets, is quite a lot. Sure, if said file does not have @tailwind directive, it will get wrapped in purgecss ignore block, but extraction of used rules from html and js will still be done from scratch for every one of those tiny stylesheets.

This PR changes the plugin to completely skip purgecss in layers mode if the source file did not include any tailwind layers. For that, I had to switch from postcss-purgecss plugin to calling purgecss manually — it does not seem like there is a way to decorate postcss-purgecss plugin to call it conditionally for both PostCSS 7 and 8. I guess, the upside of this is that latest purgecss could also be used in compat build.

Some benchmarks of this PR on our codebase:
Before: pnpm build 421.86s user 29.51s system 169% cpu 4:25.98 total
After: pnpm build 285.95s user 16.38s system 214% cpu 2:21.21 total

As you can see, it saves 2 minutes for us. It might be also very useful for other people in similar situations: integrating tailwind into existing codebase with modularized css.

In layers mode, skip `purgecss` completely if source stylesheet does
not have any tailwind layers. For the legacy codebases with a lot of
non-tailwind stylesheets, it dratically improves the performance of
the production build.
@SevInf
Copy link
Contributor Author

SevInf commented Jun 24, 2021

@adamwathan is there anything I can do to get this PR merged? This is quite an important issue for us and I'd be happy to do any changes you deem necessary.

@RockSharabShrestha
Copy link

RockSharabShrestha commented Jun 24, 2021

@adamwathan
Copy link
Member

adamwathan commented Jun 24, 2021

Just haven't had time to review, have higher priorities unfortunately. Will try to look soon, have to be really careful with changes like this though so it takes a bit of a commitment to get through it. Hopefully soon, can you use your own fork though in the mean time?

@bradlc
Copy link
Contributor

bradlc commented Jun 25, 2021

Hey @SevInf. Comparing this to PurgeCSS itself, are we missing these few lines?

if (this.options.variables) {
  this.variablesStructure.safelist = safelist.variables || [];
}

@SevInf
Copy link
Contributor Author

SevInf commented Jun 28, 2021

@bradlc this code is taken from purgecss-postcss plugin and there those lines are also missing. That, however, does not look intentional to me, so I'll add them in a moment.

@SevInf
Copy link
Contributor Author

SevInf commented Jun 28, 2021

Hey @bradlic, so I am back with the fix. You were right, those lines were missing and were breaking safelist.variables functionality. This is also something that was already broken when using purgecss-postcss. Along with the fix I've also added 2 tests: one for purging the variables and another for respecting safelist for variables. You can see that the latter one also fails on master branch.

@adamwathan
Copy link
Member

adamwathan commented Jul 1, 2021

@SevInf thanks man will put this out in the next patch!

@adamwathan adamwathan merged commit 2166b76 into tailwindlabs:master Jul 1, 2021
3 checks passed
@adamwathan adamwathan mentioned this pull request Jul 2, 2021
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

Successfully merging this pull request may close these issues.

None yet

4 participants