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

Option to not remove the space before/after CSS variables #98

Closed
saadeghi opened this issue Mar 1, 2022 · 6 comments
Closed

Option to not remove the space before/after CSS variables #98

saadeghi opened this issue Mar 1, 2022 · 6 comments

Comments

@saadeghi
Copy link

saadeghi commented Mar 1, 2022

I want to minify this

:root {
  font: var(--font-size) / var(--line-height) var(--font-family);
}

to this (see the spaces between variables)

:root{font:var(--font-size) / var(--line-height) var(--font-family)}

but currently all spaces are being removed

:root{font:var(--font-size)/var(--line-height)var(--font-family)}

[example link]

Why does it matter?

Because there's an issue from postcss-values-parser and the currently only workaround is to add spaces before and after the / which is between CSS variables.
If I minify my CSS using Parcel CSS and someone use postcss-values-parser on it, it will cause error.

I'm just wondering if there's a way to keep that space between variables.

@devongovett
Copy link
Member

hmm, according to the spec, spaces are only required around the + and - operators, not * and /. See https://drafts.csswg.org/css-values/#calc-syntax:

In addition, whitespace is required on both sides of the + and - operators. (The * and / operators can be used without white space around them.)

The postcss issue you linked to has a comment saying it is fixed:

This should be fixed as we switched to a different value parser.

Maybe you just need to upgrade to a newer version?

@saadeghi
Copy link
Author

saadeghi commented Mar 1, 2022

You're right.
postcss-custom-properties changed their parser but issue still exists in postcss-values-parser (link) which has 5M weekly downloads and so many dependents
My concern is not getting that error on my own build but when I publish my package I don't want the users get the error just because of a dependency of a dependency... (here's an example of that issue on my package) and the only workaround I found was to add spaces.

Currently I'm using clean-css. Since clean-css doesn't remove the spaces and CSSnano has an option to remove it or not, I was wondering if there's a way to make it work on Parcel CSS too.

Honestly I can't think of any other use case for it other than the workaround for the postcss-values-parsers issue so feel free to close this issue if it's an unnecessary feature.

@devongovett
Copy link
Member

if you're publishing an npm package, you might not need to minify it ahead of time? you could leave it to consumers to minify? spaces are always added if the minify option isn't enabled.

otherwise, I'll have to think about it. maybe we can just send a pr to postcss-values-parser to fix it.

@romainmenke
Copy link

romainmenke commented Mar 7, 2022

Honestly I can't think of any other use case for it other than the workaround for the postcss-values-parsers issue so feel free to close this issue if it's an unnecessary feature.

We had too many issues in postcss-preset-env that originated from postcss-values-parser.
We recently migrated everything away from it and now use https://github.com/TrySound/postcss-value-parser

I would suggest not adding more workarounds for this single package.
Currently it got so bad we would need to rewrite PostCSS itself just to keep that package working :/

@romainmenke
Copy link

romainmenke commented Mar 8, 2022

but issue still exists in postcss-values-parser (shellscape/postcss-values-parser#121) which has 5M weekly downloads and so many dependents

Most of the downloads in postcss-values-parser are because it was included in many older versions of plugins packed in postcss-preset-env. This number is now going down as people update to newer versions of postcss-preset-env.

Screenshot 2022-03-08 at 08 30 15

The downloads of later versions are negligible :

Screenshot 2022-03-08 at 08 31 35

@saadeghi
Copy link
Author

saadeghi commented Mar 8, 2022

Thanks for the details @romainmenke
So I think it's better to just ignore that package.
I'm closing this. thanks everyone 🙌

@saadeghi saadeghi closed this as completed Mar 8, 2022
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

No branches or pull requests

3 participants