geelen / jspm-loader-css

50 stars 27 forks source link

Add an option to produce a separate CSS file (fix #6) and minification (fix #7) #49

Open oligot opened 8 years ago

oligot commented 8 years ago

By default, CSS is inlined into the JS bundle and injected on execution. This commit adds an option (separateCSS) that allows to produce a separate CSS file, as available in the main CSS plugin[1]. The code is heavily influenced by the main CSS plugin, so it also uses clean-css[2] under the hood to produce the CSS file.

One nice side effect of using clean-css is that we can also optionally minify the CSS, using the Jspm CLI option --minify/-m

jspm bundle ... --minify

CSS source maps are also supported thanks to clean-css.

[1] https://github.com/systemjs/plugin-css [2] https://github.com/jakubpawlowicz/clean-css

peteruithoven commented 8 years ago

It might be interesting to pull request this to MeoMix's fork: https://github.com/MeoMix/jspm-loader-css

MeoMix commented 8 years ago

I'd happily take it, but it's worth noting that I don't have many plans to use clean-css specifically for my fork.

http://cssnano.co/ would appear to be the correct way to go since it's a PostCSS plugin.

I explored using CSS Nano in JSPM 0.16, but JSPM wasn't able to cope with it very well. However, those issues should be resolved in JSPM 0.17 and I can double-check that sometime this week. Right now I'm working on messing around with some stuff in css-modules-core.

oligot commented 8 years ago

@MeoMix What's the status of your fork ? Do you intend to merge it upstream or will it continue to diverge ?

I'll have a look at CSS Nano. Did you remember what was the problem with Jspm 0.16 ?

Out of curiosity, what are you working on css-modules-core ?

MeoMix commented 8 years ago

Hey,

Well, @geelen hasn't responded to any of my messages on Gitter for the past few weeks. He's MIA. So, I don't know if/when any of my changes will be merged upstream. I know he was looking for maintainers previously. So, maybe he'll just let his one grow stale and I'll take over more fully.

CSSNano has dependencies on Autoprefixer which wasn't working well because it was loading json dependencies from caniuse-db without specifying their file ending. JSPM would assume .js and that made life difficult. 0.17 is more easy to configure, and prevents JS extensions by default, so it was possible to get Autoprefixer working.

However, the more elegant solution was to submit a PR to Autoprefixer encouraging the use of file extensions in the require statements. Once that PR is accepted then Autoprefixer should work fine in 0.16 and the hardcoded Autoprefixer file in jspm-loader-css can be deleted.

In css-modules-core I'm looking at implementing a simple mixin plugin similar to the values/composes plugins. I don't like how composes works really at all. It feels like extends 2.0. It has some pretty bad edge cases (doesn't work in pseudo-selectors & ambiguous results if two dependencies are loaded in different orders) neither of which are issues with mixins. I'm really new to PostCSS though so it's a bit of a challenge to get something working. Hopefully I'll have it done today.

oligot commented 8 years ago

Hi @MeoMix,

I just show that your PR about Autoprefixer has been merged and is available in version 6.3.2 (https://github.com/postcss/autoprefixer/issues/618) Does it mean that we can now delete the hardcoded Autoprefixer file in jspm-loader-css ? And if yes, does it also mean that we can use CSSNano with Jspm 0.16 ?

MeoMix commented 8 years ago

Yup. Should work alright now.

oligot commented 8 years ago

Ok, I'll try another PR with CSSNano instead of clean-css

MeoMix commented 8 years ago

Sweet! Let me know how you get on.

On Thu, Feb 11, 2016 at 11:13 AM, oligot notifications@github.com wrote:

Ok, I'll try another PR with CSSNano instead of clean-css

— Reply to this email directly or view it on GitHub https://github.com/geelen/jspm-loader-css/pull/49#issuecomment-183015266 .

oligot commented 8 years ago

And here is the PR: https://github.com/MeoMix/jspm-loader-css/pull/2