-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
css optimization skipped for single module builds #176
Comments
I can contribute a fix for this bug if that would help. Though I wouldn't mind an opinion first from a project veteran on whether to just stop respecting the optimizeCss check (feels like a BC break to me), or keep respecting that but then add a new option to override that (feels bloaty). Any other way to fix while leaving optimizeCss? |
A fix for this would be welcome. Perhaps let's just always optimize the CSS if possible? |
Ok, can do, thanks @guybedford. Think I should add a new option to take its place (that r.js won't change), so that if anyone does get burned by it we can just say "ah, just change it to this option and you're good to go"? Not sure if anyone's depending on non-optimized css or not. I'll look to take care of this next week. |
Sure, if you think it's important. |
Thanks for looking into this! |
Fix guybedford#176 Only perform optimization on the css content if the build's config.css.optimize option is not set to false. This is a new config option, added under require's "config" option which allows specifying module-specific configuration without polluting the global config object.
Added usage documentation to the readme for the option to enable/disable css optimization, for fixing guybedford#176
Hi @guybedford I am facing this exact issue. Will the merge happen soon? |
Is there a workaround for this? I am being bit by this issue |
Me as well |
I've recently joined to this module maintainer, and I reviewed this issue and dedicated PR. I kinda have some doubts, but still, I will research the problem deeper these days. @darrenng-dpodium , @dingus9 , @Txabs if you have some new ideas in this area, keep us posted. |
When using
r.js
to optimize a single module using thename
andout
options,require-css
doesn't do css optimization.The compress(css) function in
css-builder.js
checks foroptimizeCss
and skips optimization if that's set to'none'
. However, r.js will overwrite optimizeCss with 'none' if anout
option was specified with nocssIn
option, with the reasoning that if you're doing a single module and it's not a css file then do no css optimization. That logic is great for normal r.js behavior, but for require-css it's likely that single js module you're optimizing contains some css you'd like to optimize as well.Perhaps we need a different option to check instead of the existing r.js optimizeCss option since r.js does some strange logic with that one? And if that's the case, how is that married with the existing check on optimizeCss since presumably people are relying on that option being checked?
The text was updated successfully, but these errors were encountered: