guybedford / require-css

A RequireJS CSS loader plugin to allow CSS requires and optimization
MIT License
983 stars 364 forks source link

Support both csso v1 and v2 #219

Closed prantlf closed 7 years ago

alundiak commented 7 years ago

csso releases review:

I did deep research of how your suggested change can affect potential end customers/consumers of require-css.

And here is my code change suggestion:

try {
  if (typeof csso.minify === 'function') {
      var minifyResult = csso.minify(css);
      if (typeof minifyResult === 'string'){ // for csso < 2.0.0
        css = minifyResult; 
      } else if (typeof minifyResult === 'object'){ // for csso >= 2.0.0
        css = minifyResult.css; 
      } 
  } else { // justDoIt() was always. minify() appeared in csso 1.4.0.
    css = csso.justDoIt(css);
  }
}

I did this, because I know enterprise projects, which still use old version of require-css and csso, so my approach is universal. Tested with:

Note: I realize, this change may be overthinking/overcomplex, but so far the fact is - if we merge ur current code change, someone who will upgrade to new require-css version, but remain old csso version, they will have errors like I mentioned in this PR and in #203.

So @prantlf please consider my code above as a suggestion to apply to your PR. After 1 week silence, I will close this PR, and recreate new one with my suggested code.

cc/ @guybedford