Merged JS/CSS improvements

[colinmollenhour]

In Magento 1.x I made some improvements to Mage_Core_Model_Design_Package. Seems like now is a good time to suggest these for 2.0:

• Cached latest mtime of all files to be merged (I see 2.0 does this, but in a .dat file.. why not the Magento cache?)
• Added a cache type for said cache records (so that Cache Management can be used to clean just the mtime records without affecting the rest of the cache)
• Added a separate base url for merged files vs media files. This is because CDNs are generally not reliable and if your merged CSS or JS doesn't load your page is broken. We serve merged files directly and all other media files from a CDN to prevent this.
• The file names are hashed once for the mtime cache record, but the final merged file name is an MD5 hash which includes these pieces of information which ensures that merged files never need to be purged from an upstream cache or browser:
  • Mage::getBaseUrl('skin') in case the skin url is updated
  • The hash of the file names (re-use mtime cache key)
  • The most recent mtime in case one of the files is updated
  • IMPORTANT: The request protocol (http or https) to prevent browser security warnings on https pages
• If the file exists ending with -min then this file name is returned instead. This allows use of an external script to minify the JS or CSS whithout overwriting the original file which may be cached upstream before it is minified.
• Never delete the parent directory of the merged files, just the contents. This way an inotify script can watch the directory and minify files as they are created. Deleting the parent directory breaks the inotify watches.
• For CSS image url replacement use the real skin base url rather than the web base so skin assets can be served from CDN.

You can see my extended Package.php with these features here: https://gist.github.com/2364485

[akira28]

I love this: Added a separate base url for merged files vs media files.
It's really affecting my job at every release, because Akamai it's unreliable for this kind of things

[magento-team]

@colinmollenhour
Thank you for all the suggested improvements.

Integration with CDN is not foreseen in current implementation of Magento 2.x Mage_Core_Model_Design_Package, but that's a must-have for the first 2.0 stable release. While storing .dat-files locally makes sense 1-server installation, the multiple web-nodes installation indeed will require a shared storage, like cache. We'll make sure to address all the issues that you pointed out, when we get to implementing Magento 2.x compatibility with CDN.

Mage::getBaseUrl('skin') in case the skin url is updated
IMPORTANT: The request protocol (http or https) to prevent browser security warnings on https pages

That's true for Magento 1.x implementation, but Magento 2.x merging algorithm deliberately doesn't prepend any base URL to the references in CSS-files, so that all relative URLs remain relative after merging. Keeping relative URLs makes sense also when the merged CSS-file ends up uploaded to CDN, as long as the structure of related resources is maintained properly.

If the file exists ending with -min then this file name is returned instead.
Never delete the parent directory of the merged files, just the contents.

Sure, we'll take it into account. To get this done quickly, you have an opportunity to contribute a pull request with these improvements, compatible with Magento 2.x code base. We'd appreciate if the provided improvement is also covered with tests. Right now the model is covered by Mage_Core_Model_Design_PackageTest (dev/tests/integration/testsuite/Mage/Core/Model/Design/PackageTest.php) and the merging algorithm tests are carried out into a separate one for convenience: dev/tests/integration/testsuite/Mage/Core/Model/Design/PackageMergingTest.php

[colinmollenhour]

@mage2-team Thanks for considering my proposals. After I have become more familiar with Magento 2 and if I have time I will work on a pull request.

While I see the merit of using domain-relative urls, I think it is better to keep the absolute urls in the CSS files and just generate separate CSS files for http and https because:

• It is necessary to be able to have different merged and skin base urls.
• It is possible that the skin urls have different base paths for http and https.

Consider these examples:

• unsecure_merged_url: http://www.example.com/media/
• unsecure_skin_url: http://d53klk224.cloudfront.net/media/

In this case with my proposal the merged url would be served from www.example.com but the skin images referenced in the CSS file should be loaded from Cloudfront. This really is a big deal since these CDNs simply are not 100% reliable (especially MaxCDN which is very popular) and if the merged JS or CSS file doesn't get loaded your site does not function. With relative URLs in the merged files you either have to have the css images loaded from the merged url or the merged files loaded from the skin url.

Consider this case:

• unsecure_skin_url: http://cdn1.example.com/skin/
• secure_skin_url: https://secure.example.com/site1/skin/

While I personally would avoid this configuration, I think it should be supported if possible. Using relative urls would break this case, methinks.

[magento-team]

Colin, thank you for describing the additional use cases. We'll make sure to take them into account when get to implementation of the CDN support.
In the meantime we'll close this ticket. Feel free to reopen it if you come up with a pull request with improvements.

[alistairstead]

Just reading through this ticket. I'm wondering about some of the comments regarding not considering CDNs or multi-node environments?

This is of course down to individual developer implementation but why would you create even a first release that has not considered the requirements of being run in a multi-node environment?

I'm sure this is not the case but that is how some of the comments read.