Suggestions
Permalink Browser Info Environment
Hi. I just came across these utilities. Very cool and something that would be nice to be included in the core.
In fact, I had started putting together something with the same goal. However, my implementation was a bit different as I was trying to require as little impact as possible. I'm probably not going to continue with mine, and you might find the following useful.
First, I was planning to create a new view library so that I could take over the getHeaderItems function. c5 is already using addHeaderItems and getHeaderItems, so I thought that it was important to allow that to continue.
In getHeaderItems I took the unique array of CSS and JS header items, added in relevant version numbers, sorted alphabetically, then hashed. This achieved several things which, from glancing at the code, it seems aren't fully handled. First, I'm using the same unique array method that c5 does in order to remove duplicate header items. Second, I've been burned by overzealous caching in the past, and I wanted to avoid the upgrade of c5 (or a package, or a block) which has new CSS/JS which doesn't get included in the cache. But your use of modtime's is probably better. Finally, I end up with a hash that represents the exact files requested, and, when the same files (with same version #s) are added in the future, from any other page, the hash should come out to be the same.
The last point seems to be the key difference. I notice you're using HTTP_HOST as a key. I don't think this will handle pages that load different sets of files. Or even the same page that loads different files depending on the context. For example, I've seen people who only add jquery when in admin mode, using an if statement. Same site, same page, different JS files. Using a key based on the site name (or requiring the developer to think of keys in the template) might cause unexpected problems. Also, think of instances in which a block might add some CSS and want it to be combined into the single file.
Lastly, I think that add-ons like yours make it more clear why c5 needs to be able to allow and handle dependencies decently. This idea is great, but of somewhat limited use if other add-ons can't take advantage of it when writing their code. I'd be more than happy to mandate that these utilities are installed, and use the AssetOptimizer to include items. However, as per c5 rules, I can't. Instead, it's up to the site owner to hand-edit every add-on to take advantage of AssetOptimizer.
In fact, I had started putting together something with the same goal. However, my implementation was a bit different as I was trying to require as little impact as possible. I'm probably not going to continue with mine, and you might find the following useful.
First, I was planning to create a new view library so that I could take over the getHeaderItems function. c5 is already using addHeaderItems and getHeaderItems, so I thought that it was important to allow that to continue.
In getHeaderItems I took the unique array of CSS and JS header items, added in relevant version numbers, sorted alphabetically, then hashed. This achieved several things which, from glancing at the code, it seems aren't fully handled. First, I'm using the same unique array method that c5 does in order to remove duplicate header items. Second, I've been burned by overzealous caching in the past, and I wanted to avoid the upgrade of c5 (or a package, or a block) which has new CSS/JS which doesn't get included in the cache. But your use of modtime's is probably better. Finally, I end up with a hash that represents the exact files requested, and, when the same files (with same version #s) are added in the future, from any other page, the hash should come out to be the same.
The last point seems to be the key difference. I notice you're using HTTP_HOST as a key. I don't think this will handle pages that load different sets of files. Or even the same page that loads different files depending on the context. For example, I've seen people who only add jquery when in admin mode, using an if statement. Same site, same page, different JS files. Using a key based on the site name (or requiring the developer to think of keys in the template) might cause unexpected problems. Also, think of instances in which a block might add some CSS and want it to be combined into the single file.
Lastly, I think that add-ons like yours make it more clear why c5 needs to be able to allow and handle dependencies decently. This idea is great, but of somewhat limited use if other add-ons can't take advantage of it when writing their code. I'd be more than happy to mandate that these utilities are installed, and use the AssetOptimizer to include items. However, as per c5 rules, I can't. Instead, it's up to the site owner to hand-edit every add-on to take advantage of AssetOptimizer.
| Type: | Discussion |
|---|---|
| Status: | New |
I had not fully investigated the integration of the core files/methods as yet. However I am aware that the optimizer would have provide even more benefit if it were to be able to handle the files which were loaded via those calls as well. It is something which I had planned to integrate in another version, I just haven't gotten the time to make any real changes to the system as yet.
The point you made about a possible conflict in the information that is generated is an issue which I had faced and worked around on my client portal. I must admit the work around was not the most elegant, as such the actual fix is higher on the list of changes I had planned on making for the next update. The current workaround however is in version 1.0.5. If you take a look at the AssetOptimizer::compile[CSS/JS] methods, there is a 'unique_key' parameter. This allows the output file to be changed on sites where multiple files are needed for various themes, or user requirements. I believe however using the actual contents in the filename may be a better way of overcoming those issues like you had suggested. I was using the HTTP_HOST variable to have the default output filename vary across servers. That will be updated in future versions. I’ll do some testing and modifications and hopefully a new version can be released soon.
I wouldn’t say the caching system I have in place is the best since sliced bread, but I think it’s rather effective. There are still some tweaks which need to be made but it hasn’t caused me any problems since I had first devised it.
I am glad I could make a contribution and I hope that with the help of everyone it can be made everyone. The code’s free for use and modifications, so if anyone thinks they have a better way of doing something feel free to modify the code or point it out. Concrete5 is a great platform and over time it will only get better. Thanks for your enthusiasm in recommending it for complete integration. It would be an honor to have it considered for inclusion in the core package, before that happens though there’s still some work to be done.
I have to say a number of improvements came about with some help from mnkras while the package was in its approval stage, so he is due some credit also. Once again thanks to everyone.