mariocasciaro / gulp-concat-css

Concatenates css files, bubbling up import statements (as per the standard), and optionally rebasing urls and inlining local import statements.
MIT License
78 stars 19 forks source link

Rebasing and import-inlining should be separate plugins. #18

Closed namuol closed 9 years ago

namuol commented 9 years ago

Given the plugin's name, it's purpose should be combining multiple files into compliant CSS, which essentially just means bubbling @import statements to the top of the file.

The rebasing feature is actually causing problems in my workflow, but it is inextricable from the rest of the plugin. I'm forced to trick the plugin into seeing my CSS files as having a different base path.

You could also just add options to disable these features, but that alone suggests these could be better-suited as separate plugins, according to the gulp plugin guidelines:

Your plugin should only do one thing, and do it well.

  • Avoid config options that make your plugin do completely different tasks
  • For example: A JS minification plugin should not have an option that adds a header as well
mariocasciaro commented 9 years ago

URL rebase and import are two operations that are complementary to concatenation. If you concatenate different CSS coming from different locations you want URL rebasing 90% of the time, same for files containing import statements. Another advantage of combining those operations is performance, if you use different plugins you have to parse the CSS 3 different times, which is a complete waste of time. I understand the gulp guidelines, but the plugin has a very simple code, it works well, is simple to use and doesn't require to execute duplicate operations. This said, I understand that some people don't want those additional operations, so I'm inclined to disable them using a configuration option rather than writing 3 different plugins. Would that work for you?

namuol commented 9 years ago

After some fiddling I managed to find a solution combining gulp-concat with gulp-minify-css.

It turns out that so long as I enable import-inlining inside gulp-minify-css, I don't have to worry about import statement bubbling, and so any number of files can be concatenated with plain-old concat(...) before being piped into minify.

However, in the case where import statements need to remain as-is, gulp-minify-css will fail, because it doesn't know to bubble import statements to the top. Sounds like I should submit an issue with gulp-minify-css instead...

But thanks! I do think the options to disable might be a good compromise for those who ever need to do rebasing or import-inlining separately, but I'm just not sure how important of a feature it is since I'm the only one who's needed it, and I've found a viable alternative.

gregbty commented 9 years ago

+1 This is causing issues in my workflow. I am forced to re-base the urls myself using gulp-replace.

mariocasciaro commented 9 years ago

Options to enable/disable rebasing are available in 2.1.0

gregbty commented 9 years ago

Thanks, works great!