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

update: use PostCSS 8 API. #125

Merged
merged 3 commits into from
Jan 3, 2021
Merged

Conversation

ludofischer
Copy link
Collaborator

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, the transform() function will exit immediately.

@Semigradsky
Copy link
Member

.travis.yml should be updated.

src/index.js Outdated
Declaration(node, {result}) {
transform(node, "value", options, result);
},
AtRule(node, {result}) {
Copy link
Collaborator

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)?

Copy link
Collaborator Author

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)) { ... }

Copy link
Collaborator

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

@ludofischer
Copy link
Collaborator Author

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?

@ludofischer
Copy link
Collaborator Author

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 calc() property. I guess the 'right' solution would be to skip writing the value if we detect the parsed value AST are equivalent? Or just use Once() :-)?

@alexander-akait
Copy link
Collaborator

@ludofischer Let's use Once

* BREAKING CHANGE: require PostCSS 8 and Node >= 10..
* Move postcss to peer dependencies.
@alexander-akait
Copy link
Collaborator

@Semigradsky Can you help and do major release due cssnano/cssnano#975 (comment)?

@Semigradsky Semigradsky merged commit 6826ba5 into postcss:master Jan 3, 2021
@Semigradsky
Copy link
Member

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.

3 participants