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

Wrong minification on invalid input #402

Closed
felixmosh opened this issue Aug 20, 2017 · 7 comments
Closed

Wrong minification on invalid input #402

felixmosh opened this issue Aug 20, 2017 · 7 comments

Comments

@felixmosh
Copy link

felixmosh commented Aug 20, 2017

Hi,

I have custom strings inside my css,

body { border: "unit(--borderWidth, px)" solid "color(color-1)"; }

CSS-Nano removes the strings after minification, and it becomes

body{border:solid}

I`ve tried, and if the input is only:

body { border-width: "unit(--borderWidth, px)"; }

The output will remain that string.

body{border-width:"unit(--borderWidth, px)"}

Expected result, those css strings should remain at the minified result.

@andyjansson
Copy link
Contributor

Sorry, we expect you to feed valid CSS to cssnano. Minification should be done as the final step in your CSS pipeline.

@felixmosh
Copy link
Author

Yeah, but why it remove the string? if it not familiar with that it suppose to keep it...

@andyjansson
Copy link
Contributor

Because the code makes executive decisions based on known keywords and CSS structure in order to optimise the values. You can disable this behaviour by disabling orderedValues.

It's hard to determine what the code is supposed to do given that the input is invalid, and attempts to handle it would be somewhat ambiguous (e.g. should the first value be a width, style, or a colour?). If you believe that it should be handled differently from how it works currently, you're welcome to state your case, and if possible, submit a PR.

@felixmosh
Copy link
Author

Can you point the place of the minification logic?

@andyjansson
Copy link
Contributor

felixmosh added a commit to felixmosh/cssnano that referenced this issue Aug 22, 2017
@felixmosh
Copy link
Author

felixmosh commented Aug 22, 2017

@andyjansson, PR is ready, WDYT?

@andyjansson
Copy link
Contributor

Seeing as we've landed on not changing the current behaviour (#403), I'm closing this issue.

@felixmosh Thank you for contributing to the project. Even though your contribution ended up being rejected, your involvement is very much appreciated. Unfortunately, sometimes we have to reject PRs in order to keep a certain level of maintainability. I hope this doesn't discourage you from contributing in the future.

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

2 participants