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

Add PurgeCSS #84

Merged
merged 2 commits into from
Oct 26, 2019
Merged

Add PurgeCSS #84

merged 2 commits into from
Oct 26, 2019

Conversation

Annihil
Copy link
Contributor

@Annihil Annihil commented Oct 20, 2019

Hey,

I am using tailwindcss those days and purgeCSS is more commonly used than uncss. I decided to go ahead and try to replace it to see how it goes. The results are quite similar when it comes to removing unused CSS although uncss can work better in some circumstances. Uncss is a bit slower on my laptop, the tests run at 970ms for uncss and about 75ms for purgeCSS.

Let me know what you think about the change.

I remove uncss as dependencies in the process. I also removed object-assign. Object.assign is available in Nodejs 4+ so I'm not sure if it's still necessary to keep this dependency. I updated the other dependencies as well.

@maltsev
Copy link
Member

maltsev commented Oct 23, 2019

Hi Annihil,

Thanks for your PR! You're totally right about the speed. I just checked both tools on a big file, purgecss is x10 faster. Though, making this migration would break old removeUnusedCss config since it's directly passed to uncss. So I'm not sure it's worth doing that.

I think the best way to deal with that is to add an option to removeUnusedCss module to change the underlying tool (uncss or purgecss), so a user could decide themself what to use. What do you think?

@maltsev
Copy link
Member

maltsev commented Oct 23, 2019

Object.assign is available in Nodejs 4+ so I'm not sure if it's still necessary to keep this dependency. I updated the other dependencies as well.

Thanks! Can you please make a separate PR for that, so I could merge it without waiting for the uncss change to be ready? By the way, as far as I remember there is no need to close an old PR and open a new one if you changed your code. Just push-force to the same branch and the PR's code would get updated automatically.

@Annihil
Copy link
Contributor Author

Annihil commented Oct 25, 2019

Hi maltsev,

Yes, I think it would be problematic for current users that are using uncss with the config. It would require a major version bump. I think your suggestion works, they would be able to use one or the other. And when it is time for a new major version, the decision to remove one or the other can be made.
I’ll send another PR for the object-assign.

@maltsev maltsev merged commit b0fc3ca into posthtml:master Oct 26, 2019
@maltsev
Copy link
Member

maltsev commented Oct 26, 2019

Great! Thanks for making these changes so quickly!

@maltsev maltsev changed the title Replace uncss by purgecss Add PurgeCSS Nov 9, 2019
@maltsev
Copy link
Member

maltsev commented Nov 9, 2019

I released this fix in the new version 0.2.5.

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.

2 participants