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

Remove dependency on postcss #267

Merged
merged 1 commit into from
Dec 3, 2018

Conversation

jbraithwaite
Copy link
Contributor

@jbraithwaite jbraithwaite commented Nov 29, 2018

postcss is a pretty heavy weight dependency if it is only being used for parsing an AST.

Replaced with css-tree.

})

value = csstree.generate(ast).slice(name.length + 1);
value = value.slice(0, value.length - 1);
Copy link

@squirly squirly Nov 30, 2018

Choose a reason for hiding this comment

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

You may want to extract 302-317 to a function. Similar to filterCss except that it could not return the new value.

Copy link
Member

@boutell boutell left a comment

Choose a reason for hiding this comment

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

csstree appears to still be in alpha, how stable is it? This module is a production dependency of a lot of projects.

@jbraithwaite
Copy link
Contributor Author

From their readme:

NOTE: The project is in alpha stage since some parts need further improvements, AST format and API are subjects to change. However it's stable enough and used by packages like CSSO (CSS minifier) and SVGO (SVG optimizer) in production.

npm reports 8 million downloads a month for this package and it is depended on by these packages (including SVGO and CSSO).

@jbraithwaite
Copy link
Contributor Author

I can write additional tests if you think that's required.

@boutell boutell merged commit e91765c into apostrophecms:master Dec 3, 2018
boutell pushed a commit that referenced this pull request Dec 5, 2018
This reverts commit e91765c, reversing
changes made to bec8ad2.
@boutell
Copy link
Member

boutell commented Dec 5, 2018

This has been rolled back for now (1.19.3), pending further examination of #269, please take a look. Thanks!

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

3 participants