-
Notifications
You must be signed in to change notification settings - Fork 32
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
update: use PostCSS 8 API. #125
Conversation
|
src/index.js
Outdated
Declaration(node, {result}) { | ||
transform(node, "value", options, result); | ||
}, | ||
AtRule(node, {result}) { |
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.
hm, maybe we should remove this from major release? What is use case for at-rule(s)?
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.
Since there is a mediaQueries
option, I think the use case for at-rules is calc()
in media queries, like:
@media (max-width: calc(40em * 2)) { ... }
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.
hm, it sounds reasonable, I think we should enable it by default
I would like to add an integration test with other plugins on realistic CSS, to check that processing terminates without errors. But I am unsure what plugins and sample CSS to pick. Maybe autoprefixer? |
I think this pull request should not be merged in this state, there's a chance of infinite loops apprearing. I've added a test that produces an infinite loop: adding a plugin that changes whitespace inside the |
@ludofischer Let's use |
* BREAKING CHANGE: require PostCSS 8 and Node >= 10.. * Move postcss to peer dependencies.
d126bea
to
c75eee9
Compare
@Semigradsky Can you help and do major release due cssnano/cssnano#975 (comment)? |
I've noticed cssnano/cssnano#954 assumes there's a version of this with the PostCSS 8 API, but I could not find it either a release, pull request or branch, so here it it.
I've used the visitor API thinking it should be safe from infinite loops, as once the
calc()
declarations are gone, thetransform()
function will exit immediately.