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

Minify fails with escaped less code #13604

Closed
robertdodd opened this issue May 15, 2014 · 8 comments
Closed

Minify fails with escaped less code #13604

robertdodd opened this issue May 15, 2014 · 8 comments
Milestone

Comments

@robertdodd
Copy link
Contributor

I want to use escaped text in my less code:

.timeline {
    width: e("calc(100% - 90px)");
}

But because css is minified using less:clean-css, the code in bootstrap.css gets calculated again and comes out in bootstrap.min.css as:

.timeline {
    width: 10%;
}

I thought of telling the less:minify task in the gruntfile to use the original bootstrap.less file as the source, but then it loses out on the autoprefixer and csscomb tasks that usually run on the css beforehand.

Any ideas on how to overcome this?

@cvrebert
Copy link
Collaborator

Sounds like a possible clean-css bug.
CC: @XhmikosR

@XhmikosR
Copy link
Member

I can't reproduce the issue with clean-css 2.1.8:

.timeline{width:e("calc(100% - 90px)")}

So it must be less that's causing this.

@seven-phases-max
Copy link

So it must be less that's causing this.

Not really a Less fault. I looked at Gruntfile.js and the minify task is looking at least strange there. Does not it compile already compiled css as less code for the second time? If so then @robertdodd 's result is expected since the first run (less-compile) compiles e("calc(100% - 90px)"); to calc(100% - 90px); and the second run (less:minify) compiles calc(100% - 90px); to calc(10px); since no --strict-math option set for this task. I guess this particular issue can be overcome by adding strictMath: true to the less:minify task, but yet again, the task itself is very questionable (Less is not really 100% compatible with CSS so such "double compilation" just opens a can of worms).

@XhmikosR
Copy link
Member

Not really a clean-css fault, that was my point.

Personally, I don't like Less' approach with its built in clean-css, and I wasn't the one who added that so I don't know if it's done right or not.

@seven-phases-max
Copy link

Not really a clean-css fault

Yes, of course.


I guess the proper solution would be to reassign the minification of those css files from the less task to the cssmin task.

@XhmikosR
Copy link
Member

@seven-phases-max: feel free to make a PR for that.

@XhmikosR
Copy link
Member

Fixed in #13624

@XhmikosR XhmikosR added this to the v3.2.0 milestone May 19, 2014
@robertdodd
Copy link
Contributor Author

Awesome, I'm glad to have helped, thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants