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 basic postcss 8 support #129

Merged
merged 1 commit into from May 11, 2021
Merged

Add basic postcss 8 support #129

merged 1 commit into from May 11, 2021

Conversation

delucis
Copy link

@delucis delucis commented Apr 5, 2021

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

index.js Show resolved Hide resolved
@@ -64,229 +63,234 @@ var defaults = {
preserveAtRulesOrder: false
};

module.exports = postcss.plugin("postcss-css-variables", function(options) {
module.exports = (options = {}) => {
Copy link
Owner

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.

Copy link
Author

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

@MadLittleMods
Copy link
Owner

Thanks @delucis! This looks like it's matching the migration guide ❤️ , https://evilmartians.com/chronicles/postcss-8-plugin-migration

@delucis
Copy link
Author

delucis commented May 11, 2021

Just in case my review comment got missed:

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

@MadLittleMods MadLittleMods merged commit 2d7e0ee into MadLittleMods:master May 11, 2021
MadLittleMods added a commit that referenced this pull request May 11, 2021
@MadLittleMods
Copy link
Owner

Thanks for the poke on this @delucis! Merged and shipped in postcss-css-variables@0.18.0 🚀

Thanks for the contribution to bring us more up to date ❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants