meenie / munee

Munee: Standalone PHP 5.3 Asset Optimisation & Manipulation
https://munee.herokuapp.com/
MIT License
833 stars 92 forks source link

@import and minification #59

Closed webbird closed 9 years ago

webbird commented 9 years ago

I have tried munee now, but I still seem to miss something.

Folder structure:

Base is a subdir under the doc root: <DOCUMENTROOT>/sub1/sub2

CSS files are placed in a subdir: <DOCUMENTROOT>/sub1/sub2/templates/freshcat/css/default

Munee is located in a different sub folder: <DOCUMENTROOT>/sub1/sub2/lib_munee/munee.php

The only file loaded is index.css, which contains a bunch of @imports.

@import url(reset.css);
@import url(webfont.css);
@import url(colors.css);
...

Result is a minified index.css, but the files that are imported are not minified. Is this by design, or do I have to tweak my config?

define('MUNEE_FOLDER','<DOCUMENTROOT>/sub1/sub2/lib_munee'));
define('MUNEE_CACHE' ,'<DOCUMENTROOT>/sub1/sub2/temp/cache'));
define('DS'          ,DIRECTORY_SEPARATOR);
define('WEBROOT'     ,'<DOCUMENTROOT>/sub1/sub2');
define('SUB_FOLDER'  ,'sub1/sub2');

Note: I had to set the SUB_FOLDER to make the paths resolve correcty.

Thanks!

meenie commented 9 years ago

Using native @import's are basically like adding <link> tags to your HTML. I believe if you just add ?minify=true at the end, it should work. Something like @import url(reset.css?minify=true);. If not, then no, it's not supported because I'm not parsing those import tags. You could try using LESS, as that actually imports the content when the file is rendered.

webbird commented 9 years ago

I have a function I got from here: https://github.com/dg/ftp-deployment/blob/v1.1/Deployment/libs/Preprocessor.php#L63

Added it to Css.php and called it in beforeFilter() like so:

        $content = file_get_contents($originalFile);
$content = self::expandCssImports($content,$originalFile);
        file_put_contents($cacheFile, $this->fixRelativeImagePaths($content, $originalFile));

Some results from the network tab in Firebug:

Without minification 24 request, between 200 and 240 ms. With minification 1 request, between 111 and 142 ms. (file size ~80 kb)

Will have to measure the time for creating the cache.

Edit: Times in local XAMPP environment.

webbird commented 9 years ago

This works quite nice. Given an index.css with 21 imports:

time diff result minifying is 0.17000007629395, without (cache) 0.1029999256134.

The time diff measuring the expandCssImports() only is around 0.020000219345093.

I think it would be better to replace the preg_replace_callback() in expandCssImports() with something else, but for now, this is working fine for me.

If you're interested in my final solution, I can make a fork an send you a pull request.

meenie commented 9 years ago

@webbird Nice find :). And yes, I'd like to see a PR of what you've come up with. If you want to go the extra mile, you could include tests as well :). Also, please make sure and document where you got that function so we are properly citing other peoples work. Or maybe there is a standalone library in Composer that can do this?

webbird commented 9 years ago

I always add the source as a comment, so I can always go back there to see what I've mixed up if it stops working. ;) I've not found a library yet, only a somewhat different solution to extract the URLs here:

http://nadeausoftware.com/articles/2008/01/php_tip_how_extract_urls_css_file

Maybe this one is a bit closer to a final solution as it seems to consider some more alternatives and doesn't use preg_replace_callback().

I am going to add Munee as an optional library to BlackCat CMS as of version 1.2 as our headline is "SEO" for this version. This is why I asked if it's still maintained. Munee seems to be exactly what I'm after. Great stuff. :D

meenie commented 9 years ago

Cool :)

webbird commented 9 years ago

I will have to put some more effort into this. While the first solution works fine, the second one takes into account optional attributes:

@import "styles.css"; @import url("styles.css"); @import url("druck.css") print; @import url("foo.css") projection, tv; @import url("bar.css") handheld and (max-width: 500px);

So @imports that have such optional attributes should be wrapped with appropriate @media blocks.

I will try to combine the solutions somehow.

meenie commented 9 years ago

That's a pretty cool concept :).

webbird commented 9 years ago

I have two solutions now. The first one is the one from here: https://github.com/dg/ftp-deployment/blob/v1.1/Deployment/libs/Preprocessor.php#L63

The second one

Both solutions have nearly the same benchmark results in my local XAMPP, though the second one tries to not use RegEx and preg_replace_callback(). I will now try to extend the first one to take media attributes into account. I can commit my dev snapshot into my fork if you'd like to take a look. The second solution may not be the most elegant one as it uses several strpos() and substr() and only one simple RegEx to extract the @imports, but I chose this to see if the performance is worth it. I am now going to take CPU usage into account, but that is somewhat tricky on Windows.

webbird commented 9 years ago

https://github.com/webbird/munee/commit/534d78de79a2958cd1ad437b4afde84b16d54120

webbird commented 9 years ago

I have now tested 3 solutions:

Here are my results:

expandCssImports Stats
START: 1425485268.2664
END:   1425485268.2976
DIFF:  0.031200170516968
MEM:   0.20252227783203

parseImports Stats
START: 1425485268.2976
END:   1425485268.3288
DIFF:  0.031199932098389
MEM:   0.11023712158203

parseImportsv2 Stats
START: 1425485268.3288
END:   1425485268.3444
DIFF:  0.015599966049194
MEM:   0.11019897460938

The solution using one regexp seems to be the fastest and least memory consuming one.

webbird commented 9 years ago

Thanks for accepting the pull request.

meenie commented 9 years ago

@webbird: I've thought of something. Does this take into consideration the caching mechanisms? If you were to @import a CSS file, refresh the page, then edit the imported CSS file, and then refresh the page again. Do you get those changes? I have a feeling that might not be the case. The caching system checks the last modified time of the original file you are linking to. Since the file you are linking to hasn't actually changed, then it's not going to invalidate reparse the @import tags.

You may have to override the Munee\Asset\Type::generateCacheFile() method in the CSS type class to take into consideration the modified times of all the files that have been imported for that particular file.

webbird commented 9 years ago

Yes, you're right. I create an issue. (In my fork.)

Edit: https://github.com/webbird/munee/issues/1

meenie commented 9 years ago

Thinking about this more.... I think that if you want this feature, you should just change your file extensions to either .scss or .less as either of those actually do what you want here. What do you think? If that's the case, I'll revert back out the changes because this fundamentally changes how vanilla CSS files work.

meenie commented 9 years ago

I've created a PR that I think addresses this: #72

I'm going to close this and we can continue discussion there.