webpack-contrib / purifycss-webpack

UNMAINTAINED, use https://github.com/FullHuman/purgecss-webpack-plugin
MIT License
772 stars 37 forks source link

Allow purifycss-webpack to work with ExtractTextCssPlugin when hash is in CSS asset name #94

Closed IAMtheIAM closed 7 years ago

IAMtheIAM commented 7 years ago

Allow purifycss-webpack to work with ExtractTextCssPlugin when hash is added to css file name.

Example: [name].css?[hash] -> style.css?218aa9358a709a5a0a12

jsf-clabot commented 7 years ago

CLA assistant check
All committers have signed the CLA.

bebraw commented 7 years ago

Can you add a test below search.test.js? The CI should pass too. Good enough after that.

bebraw commented 7 years ago

About CI, looks like an existing search test fails with the new logic so the code may require some tweaking not to break the old logic.

IAMtheIAM commented 7 years ago

The test is very strict and I don't understand what it is saying. It doesn't like curly braces around the function: Before

//passes
    name => extensions.indexOf(path.extname(name)) >= 0 && { name, asset: assets[name] }

After

//fails
    name => { 
         extensions.indexOf(path.extname(name)) >= 0 && { name, asset: assets[name] } 
}

However it won't let me add an variables or conditions in that section without curly brace, it warns { expected

//fails
    name => {
      var nameCleaned = name;
      if (/\.(css\?).*$/.test(name)) {
        nameCleaned = name.substr(0, name.lastIndexOf('?')); // ignore hash on file like style.css?7ec000f0d0d347
      }
      extensions.indexOf(path.extname(nameCleaned)) >= 0 && { name, asset: assets[name] }
    }

So I don't know how to fix it without some guidance about what the test wants.

bebraw commented 7 years ago

No probs. I'll try to figure out a fix. 👍

bebraw commented 7 years ago

Ah... If you want multiple lines, you could use name => ( ... ). That should be equivalent to a single line. With curlies it won't return by default.

codecov[bot] commented 7 years ago

Codecov Report

Merging #94 into master will increase coverage by 6.36%. The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   66.25%   72.61%   +6.36%     
==========================================
  Files           5        5              
  Lines          80       84       +4     
  Branches       25       28       +3     
==========================================
+ Hits           53       61       +8     
+ Misses         27       23       -4
Impacted Files Coverage Δ
src/search.js 95.45% <80%> (-4.55%) :arrow_down:
src/index.js 18.51% <0%> (+18.51%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a0a2087...c0bd413. Read the comment docs.

IAMtheIAM commented 7 years ago

Got it working. Still learning ES6 syntax :-) Thanks for the help.