jasonlewis / basset

A better asset management package for Laravel.
http://jasonlewis.me/code/basset
240 stars 76 forks source link

CssMin strips whitespace it shouldn't #155

Open Anahkiasen opened 11 years ago

Anahkiasen commented 11 years ago

I have a class .layout-header .layout-header__logo--small. When minified it now becomes .layout-header.layout-header__logo--small which of course fails. This wasn't happening before so I'm not sure where it's coming from, if it's from Basset or Assetic but it kind of breaks everything.

Anahkiasen commented 11 years ago

After investigation, the CssMin filter didn't change, it's JsMin that's applying itself to .css files although I specified whenAssetIsStylesheet and added my assets with ->stylesheet.

jasonlewis commented 11 years ago

You're applying the JsMin filter with whenAssetIsStylesheet?

Anahkiasen commented 11 years ago

No CssMin uses whenAssetIsStylesheet, JsMin uses whenAssetIsJavascript

jasonlewis commented 11 years ago

Can you paste your collection somewhere? I can't seem to recreate this.

Any details you can give me on your assets as well. How they're named, etc.

d13r commented 11 years ago

I have the same issue when calling both CssMin and JsMin on the same collection:

'app' => function($collection)
{
    $collection->stylesheet('css/app.css');
    $collection->apply('CssMin');
    $collection->apply('UriRewriteFilter');

    $collection->javascript('js/app.js');
    $collection->apply('JsMin');
},

Removing the JsMin line makes the stylesheet work correctly. The aliases haven't been changed from the defaults:

'CssMin' => ['CssMinFilter', function($filter)
{
    $filter->whenAssetIsStylesheet()->whenProductionBuild()->whenClassExists('CssMin');
}],

'JsMin' => ['JSMinFilter', function($filter)
{
    $filter->whenAssetIsJavascript()->whenProductionBuild()->whenClassExists('JSMin');
}],

It seems to be applying the JavaScript filters to the CSS files. I traced the problem to this line in Filter/Filterable.php:

$filter = $this->factory->get('filter')->make($filter)->setResource($this);

It looks like there is one filter object shared by multiple assets, so whichever asset comes last is used. This fixed it:

$filter = clone $this->factory->get('filter')->make($filter)->setResource($this);

But I think it might be better to replace this in Factory/FilterFactory.php:

        if ($filter instanceof Filter)
        {
            return $filter;
        }

With this:

        if ($filter instanceof Filter)
        {
            return clone $filter;
        }

Or there might be a better solution...

Marwelln commented 11 years ago

This is still not fixed. I had to add clone to the FilterFactory.php file (as @davejamesmiller mentioned) or else #foo .bar would become #foo.bar.

My collection looks like this:

    'app' => function($collection) {
        // third party css
        $collection->add(app_path().'/assets/js/third-party/colorbox/colorbox.css');

        // our (less) css
        $collection->add(app_path().'/assets/less/app.less')->apply('Less');
        $collection->add(app_path().'/assets/less/feed.less')->apply('Less');           
        $collection->apply('CssMin');

        // js
        $collection->add(app_path().'/assets/js/third-party/jquery.isotope.min.js');
        $collection->add(app_path().'/assets/js/third-party/jquery.history.js');
        $collection->add(app_path().'/assets/js/third-party/colorbox/jquery.colorbox.js');
        $collection->directory('/../app/assets/js/classes', function($collection){
            $collection->requireDirectory()->apply('JsMin');
        });
        $collection->requireTree('/../app/assets/js/lang/'.Config::get('app.locale'))->apply('JsMin');
        $collection->add(app_path().'/assets/js/app.js');
        $collection->apply('JsMin');    
    }

I build the file with php artisan basset:build app --production. Using "natxet/CssMin": "dev-master", "leafo/lessphp": "0.4.0" and "lmammino/jsmin4assetic": "dev-master" in my require.

Anahkiasen commented 11 years ago

It's fixed on develop, but apparently Basset isn't maintained anymore, which is kind of a shame as I really liked it.

jasonlewis commented 11 years ago

Feel free to fork guys. Will be doing a write up when I get a chance. On 18 Oct 2013 20:05, "Maxime Fabre" notifications@github.com wrote:

It's fixed on develop, but apparently Basset isn't maintained anymore, which is kind of a shame as I really liked it.

— Reply to this email directly or view it on GitHubhttps://github.com/jasonlewis/basset/issues/155#issuecomment-26581276 .