HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Loading JavaScript libraries with PHP API

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptphplibrarieswithloadingapi

Problem

I have written the following script which simply:

  • Loops through JavaScript files (either defined in a particular package, or separately with the libraries array)



  • Reads the files into the script and combines them into one string



  • Minifies the string and saves the contents to a cached file on the server and returns the contents.



Some key features:

  • The cached file is only updated when the script detects that one of the loaded JavaScript libraries has been modified



  • The script only gathers the files if a cached version does not exist



I am sure most of us programmers will notice that the array of libraries could very easily be built dynamically by scanning the _js/_source/ directory and therefore I would not have to add each library to the $js_libraries array individually. The reason I have not done this is simple, it is much slower when there are a lot of files and the absolute key of this script is speed!

I simply want to know whether this could be written better and whether it has any areas I could improve on.

```
'/_js/_source/_md5/sinemacula.md5.js',
'cookies' => '/_js/_source/_cookies/sinemacula.cookies.js',
'preloading' => '/_js/_source/_preloading/sinemacula.preloading.js',
'browsers' => '/_js/_source/_browsers/sinemacula.browsers.js',
'overlay' => '/_js/_source/_overlay/sinemacula.overlay.js',
'forms' => '/_js/_source/_forms/sinemacula.forms.js',
'fancybox' => '/_js/_source/_fancybox/jQuery.lightbox-0.5.js',
'indexof' => '/_js/_source/_indexOf/init.indexOf.js'
// This array will be much larger
);

// Only define the javascript packages if the
// package is not coming from an external source
if($package){
// Define all the javascript packages
$js_packages = array(
'all' => array_keys($js_libraries)
// This array will be much larger
);
// Make sure the supplied package exists
if(array_key_exists($package,$js_packages)){
// Gathe

Solution

Don't point at php

This script looks like the intention is to point at a php file, that returns the compressed content. Assuming that's the case...

PHP doesn't belong in-between a user and a static, public file. Webservers are much better for serving static content. Rather than pointing at a php script that processes the request, looks for and/or manipulates files and then serves it - take advantage of the webserver and point at the cache file directly - or where the cache file should be if it doesn't exist.

I.e. change this logic:

user -> /someurl.php?args -> php -> build/find static file -> serve it


To the equivalent of:

user -> /js/packet/hash.js


Putting as little as possible in-between the user and the content they are requesting means better performance.
Handle requests for missing packets

If you want to automatically handle requests for missing packets, add a .htaccess file (or equivalent) like this to the css/js folder:


    RewriteEngine On
    RewriteCond %{REQUEST_FILENAME} !-f
    RewriteRule ^(.*)$ handlemissingasset.php [L]


You'll have the possibility to handle requests for a packed js/css file on the first request where it's missing. Write to the equivalent path of the current url - and the webserver will take care of serving the file and sending appropriate headers.
Derive the url at run time, not the file's contents

Instead of using a url which contains, serialized in some form, the contents that are to be returned - just derive the url for the packet file. So e.g.:

function url($packets) {
    return md5(implode($packets) . APP_VERSION) . '.js';
}

echo sprintf('', url(array('foo', 'bar')));


The less logic there is at runtime - the faster the code performs.
Use a build process

Rather than handle requests for missing packets, a process that builds them all, once, could be used.

$ . buildPackets.php
Scanning for packets
... found ('foo', 'bar')
... bumping APP_VERSION
... writing js/packets/d41d8cd98f00b204e9800998ecf8427e.js


The benefit of working in this way is that the run-time logic is reduced to the bare minimum, there's no need at any point of a web request to check if files exist ect. This is also more or less a pre-requisite to be able to use a CDN.
Explicit is better than implicit

I'm not sure what the logic at the top of the code in the question is for, but (and this is from experience). if in a project there are requests for:

'foo', 'bar'
'bar', 'foo'
'foo', 'bar', 'other'
'zum', 'bar', 'other'


It becomes difficult to automatically serve optimal packets.

If each individual request results in 1 js file that is sub optimal, as the user receives one file per page, but the same content multiple times as they browser around the site.

As such it's likely better to define all js packets in a config file and only refer to the packet names rather than permit dynamic usage.
Appropriate Headers

Whether you heed or ignore the above, this point is the biggest failing with the code as presented. There are 2 ways to handle headers for asset files (css, js images), the serverver and/or the php script needs to implement correct headers for optimal performance.
Validation requests

A short expiry is sent in the headers, the user sends a request and a successful response is either a 200 or a 304 - make sure to be sending 304s where appropriate.

This means a user downloads the file once on the first request, and on each subsequent request for the same asset they receive an empty 304 not modified response. Not sending the user back the same file saves bandwidth and increases speed.
Long expiry

If the url changes when an asset changes - there's no need for validation requests and you can send in the headers that the response is valid for - e.g. a year. This means a user downloads the file on the first request - and never requests it again. Validation requests are a great improvement over no/incorrect cache headers. Long expiry is another significant improvement.
Finally: Look for existing solutions

You may find repos like this one useful, either the code, the docs, the api or all of the above. Try to avoid the mistakes that many have already made before you :).

Code Snippets

user -> /someurl.php?args -> php -> build/find static file -> serve it
user -> /js/packet/hash.js
<IfModule mod_rewrite.c>
    RewriteEngine On
    RewriteCond %{REQUEST_FILENAME} !-f
    RewriteRule ^(.*)$ handlemissingasset.php [L]
</IfModule>
function url($packets) {
    return md5(implode($packets) . APP_VERSION) . '.js';
}

echo sprintf('<script src="%s"></script>', url(array('foo', 'bar')));
$ . buildPackets.php
Scanning for packets
... found ('foo', 'bar')
... bumping APP_VERSION
... writing js/packets/d41d8cd98f00b204e9800998ecf8427e.js

Context

StackExchange Code Review Q#18117, answer score: 4

Revisions (0)

No revisions yet.