When the minification process is running it should check the modified date of the source file against the modified date of the minified file. This will improve the performance of the minifcation process- particularly for people who are using a third party remote service. This means that we need to track modification times in speedy_min_js_files. This will require refactoring speedy_js_alter().

CommentFileSizeAuthor
#2 speedy.diff12.84 KBarthurf

Comments

arthurf’s picture

Here's a stab at the drush function to base minification on modification dates. Note that there are a few tweaks here:
* I created a hacked version of file_scan_directory (speedy_file_scan_directory()) which includes a date stamp with the returned $file. Core's file_scan_directory has a callback function option but only passes the $uri to it rather than the $file object so you can't actually modify the $file that is being returned.
* I've tried to abstract this code a bit so it's easier to duplicate- or even just extend
* I haven't tested this code yet. Just working on a proof of concept here

function drush_uglifyjs_uglify_files($uglifyjs = '') {
  // Path to the minified JS library
  $minified_directory = variable_get('speedy_js_path', DRUPAL_ROOT . '/' . drupal_get_path('module', 'speedy') . '/js/');
  // Ensure that we can write to this directory
  // @todo what is the error condition here?
  $minified_directory = file_prepare_directory($minified_directory, FILE_CREATE_DIRECTORY);

  // Find all JS files
  $all_js_files = speedy_file_scan_directory(DRUPAL_ROOT, '/.js/');

  // Current minfied files
  $current_minfied_files = variable_get('speedy_min_js_files', array());

  // Start minifying files
  foreach ($all_js_files as $file) {
    // By default we do not modify this file
    $minify = FALSE;

    // Make sure the file is not blacklisted.
    if (speedy_file_blacklisted($file->uri)) {
      $minify = FALSE;
    }
    // Has this file ever been minified?
    elseif (empty($current_minified_files[$file->uri])) {
      $minify = TRUE;
    }
    // Is the original modified date more recent than the current minified file?
    elseif ($current_minified_files[$file->uri] > $file->modified) {
      $minify = TRUE;
    }

    // Should we minify this file?
    if ($minify) {
      // Create a file path to the store directory
      $filepath = $minified_directory . preg_replace("/.js/", ".min.js", basename($file->uri));

      // Now we can attempt to generate the file
      if (uglify_minify_file($file->uri, $filepath)) {
        // Add the minified filepath to the list of saved files
        $file->minified_file = $filepath;
        $current_minfied_files[$file->uri] = $file;
        drush_log(dt('!filename was minified.', array('!filename' => $file->uri)), 'success');
      }
      else {
        // @todo Note that a file was created with the error information in it.
        drush_log(dt('There was an error when trying to minify !filename.', array('!filename' => $file->uri)), 'error');
        unset($current_minified_files[$file->uri]);
      }
    }

  }

  // Save the list of minified files
  variable_get('speedy_min_js_files', $current_minfied_files);
}
arthurf’s picture

StatusFileSize
new12.84 KB

First sorry for this ridiculous patch. It's got changes all over the place. Basically this was a "add modification dates" patch and then suddenly snowballed. I've moved all the core functionality into speedy_minify() This allows you to call this function with your own callback to implement minification. I don't like this so much but it allows the module to handle all the file finding, blacklisting, date checking, etc. Personally I think this is preferable to having developers implement their own mechanisms for doing this. Anyway, I'm happy to discuss some of the changes or untangle things, it just happens to be what is working in my dev environment right now.

mfer’s picture

This is an interesting idea. Though, I wonder if time is poor to use and a hash of the file should be used instead. I've seen issues where date stamps get messed up. A hash would detect a change though I realize it adds computational complexity. Thoughts?

arthurf’s picture

Seems like we could just store an md5 hash of the file rather than the modified date. In speedy_file_scan_directory() we could replace the modified date code with $files[$key]->md5 = md5_file($files[$key]->uri) and in speedy_minify() replace
elseif ($current_minified_files[$file->drupal_filepath]->modified > $file->modified)
with
elseif ($current_minified_files[$file->drupal_filepath]->md5 == md5_file($current_minified_files[$file->drupal_filepath]->uri))

That work for you?

mfer’s picture

I'm not sure. Here's why. Every time call md5_file it makes a filesystem call to generate the hash. I would like to not increate the numer of file system calls. While they may be fast, Drupal already does too many of them. I'm not entirely sure here.

arthurf’s picture

True, but these would only be done on calls to speedy_minify() during generation- which is also likely to be called from drush- not ideal I agree with you. Is that a reasonable performance compromise?