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 basic postcss 8 support #129
Add basic postcss 8 support #129
Conversation
@@ -64,229 +63,234 @@ var defaults = { | |||
preserveAtRulesOrder: false | |||
}; | |||
|
|||
module.exports = postcss.plugin("postcss-css-variables", function(options) { | |||
module.exports = (options = {}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does supporting this new syntax drop support for older versions of PostCSS? Which versions?
I want to know so I can add this to the changelog and major breaking version if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is a hard break. I tried a minimal build task with postcss@^7 and this branch and got this error:
Error: PostCSS plugin postcss-css-variables requires PostCSS 8.
Migration guide for end-users:
https://github.com/postcss/postcss/wiki/PostCSS-8-for-end-users
Thanks @delucis! This looks like it's matching the migration guide ❤️ , https://evilmartians.com/chronicles/postcss-8-plugin-migration |
Just in case my review comment got missed:
|
Thanks for the poke on this @delucis! Merged and shipped in Thanks for the contribution to bring us more up to date ❤ |
I’ve been using this branch by @larslaade for compatibility with postcss@^8 and it seems to work pretty well. Perhaps it would be good to merge it into the main package?
This would address #127.
https://evilmartians.com/chronicles/postcss-8-plugin-migration