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

@import inline broken in version 2.0.3+ #198

Closed
marrone opened this issue Dec 26, 2013 · 12 comments
Closed

@import inline broken in version 2.0.3+ #198

marrone opened this issue Dec 26, 2013 · 12 comments
Assignees
Milestone

Comments

@marrone
Copy link

marrone commented Dec 26, 2013

Commit 7b739b6 seems to have broken @imports between version 2.0.2 and 2.0.3

In our main.css file, we have something like

@import url('a.css');
@import url('b.css');
@import url('c.css');
#foo { color: blue; }

In 2.0.2, the imports were merged inline and minified without issue. In 2.0.3, only the first @import is merged, and the following @imports are not, instead the final @import appears broken inline, the above example outputs

#aaa{color:red;}import url(c.css);#foo{color:blue;}

I tried creating a simple test case, but it appears to depend partially on the contents of the imported stylesheets (I suspect a greedy regex), so I do not yet have a simple repeatable example for you

marrone referenced this issue Dec 26, 2013
* Broken comments scanner when more than one import inside a comment.
@GoalSmashers
Copy link
Contributor

@marrone Thanks for bringing this into our attention. Will try to recreate it locally first then fix it ASAP in 2.0.5.
Please use 2.0.2 until then.

@ghost ghost assigned GoalSmashers Dec 26, 2013
@GoalSmashers
Copy link
Contributor

@marrone it's indeed a bit hard to replicate it so would it be possible for you to mail your CSS files at dev (at) goalsmashers.com? We'd track down the issue, create a simple test case, and delete your files afterwards.

@janraasch
Copy link

I believe I stumbled upon the same problem. I could break down the problem to the following situation.

import1.css:

.some-css {
  color: blue;
}

import2.css:

.some-more-css {
  color: red;
}

main-with-comment.css:

@import url('import.css');
@import url('import2.css');
/*comment!*/

Now, cleancss main-with-comment.css yields .some-css{color:red}import url(import2.css);~. Everything works fine, without the /*comment!*/ and/or only one @import. I just tested this with cleancss --version 2.0.4.

@GoalSmashers
Copy link
Contributor

@janraasch that's it! Thanks for coming up with the test case. Fix should be there soon.

@GoalSmashers
Copy link
Contributor

@janraasch Just noticed the error does not go away with 2.0.2 so it may be another edge case - @marrone would it be possible to share your code to make sure we are solving one issue?

@janraasch
Copy link

I noticed the bug I was describing with the update of grunt-contrib-cssmin to 0.7.0 which was when they updated their clean-css dependency to ~2.0.0.

@marrone
Copy link
Author

marrone commented Dec 27, 2013

@janraasch grunt-contrib-cssmin was also where I encountered the issue. For me the error was introduced with 2.0.3 of clean-css. When you force the dependency of 2.0.2, it works. I am using all default options, so perhaps those defaults changed for clean-css which may be a difference in the result as well, I dont know yet

@GoalSmashers I will try to provide a simple test case

@GoalSmashers
Copy link
Contributor

@marrone Great, looking forward to it!

GoalSmashers pushed a commit that referenced this issue Jan 3, 2014
…nts.

This is a follow up to #192 which apparently fixed an edge case of processing
more than one @import inside a comment but introduced this issue.
@GoalSmashers
Copy link
Contributor

@marrone @janraasch If you have a moment and opportunity please check 904145b as it's a potential fix to this issue.

GoalSmashers pushed a commit that referenced this issue Jan 4, 2014
…nts.

This is a follow up to #192 which apparently fixed an edge case of processing
more than one @import inside a comment but introduced this issue.
@GoalSmashers
Copy link
Contributor

@marrone @janraasch clean-css 2.0.6 is out with the fix, so please verify if it works for you and feel free to reopen this issue if needed.

@janraasch
Copy link

Works fine for me. Thanks for the fix.

@GoalSmashers
Copy link
Contributor

Sure, anytime 👍

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