Merge pull request #25 from mozilla/issue_24_hash_in_url

Allow development resources to contain fragment identifiers.

lib/connect-cachify.js

@@ -64,7 +64,7 @@ exports.setup = function (assets, options) {
     }
     if (m && m.index === 0) {
       // 10 first characters of md5 + 1 for slash
-      true_path = req.url.slice(prefix.length + 11); 
+      true_path = req.url.slice(prefix.length + 11);
       exists = false;
 
       if (opts.prefix !== '' &&
@@ -120,20 +120,27 @@ exports.setup = function (assets, options) {
  * foo.js                    -> a998d8e98f/foo.js
  * http://example.com/foo.js -> http://example.com/foo.js
  */
-var hashify = function (filename, hash) {
+var hashify = function (resource, hash) {
-  if (typeof filename !== 'string')
+  if (typeof resource !== 'string')
-    throw "cachify ERROR, expected string for filename, got " + filename;
+    throw "cachify ERROR, expected string for resource, got " + resource;
-  if (filename.indexOf('://') !== -1) {
+  if (resource.indexOf('://') !== -1) {
-    return filename;
+    return resource;
   }
+
   if (! hash && opts.global_hash) {
     hash = opts.global_hash;
   }
 
   if (opts.production !== true &&
       opts.debug === false) {
-    return filename;
+    return resource;
   }
+
+  // fragment identifiers on URLs are never sent to the server and they
+  // should not exist on the filename. When looking up the resource on
+  // disk, do so without the fragment identifier.
+  var filename = resource.replace(/#.*$/, '');
+
   if (hash) {
     // No-op, specifing a hash skips all this craziness
   } else if (_cache[filename] && _cache[filename].hash) {
@@ -144,22 +151,24 @@ var hashify = function (filename, hash) {
     var md5 = crypto.createHash('md5');
     try {
       var data = fs.readFileSync(path.join(opts.root, filename));
+
       md5.update(data);
       // Expensive, maintain in-memory cache
       if (! _cache[filename]) _cache[filename] = {exists: true};
       hash = _cache[filename].hash = md5.digest('hex').slice(0, 10);
     } catch (e) {
       // Not intersting to cache, programmer error?
+      exports.uncached_resources.push(resource);
       console.error('Cachify bailing on hash... no such file ' + filename );
       console.error(e);
-      return filename;
+      return resource;
     }
   }
   if (opts.prefix.indexOf('://') === -1 &&
-      filename[0] === '/') {
+      resource[0] === '/') {
-    return format('/%s%s%s', opts.prefix, hash, filename);
+    return format('/%s%s%s', opts.prefix, hash, resource);
   } else {
-    return format('%s%s%s%s', opts.prefix, hash, filename[0] === '/' ? '' : '/', filename);
+    return format('%s%s%s%s', opts.prefix, hash, resource[0] === '/' ? '' : '/', resource);
   }
 };
 
@@ -207,3 +216,5 @@ var no_url = function (prefix) {
 var escape_regex = function (str) {
   return str.replace('/', '\/');
 };
+
+exports.uncached_resources = [];

test/connect-cachify-test.js

@@ -15,8 +15,10 @@ var resetConfig = function () {
 
 var make_assets = function () { return {
   "/js/main.min.js": [
-    "/js/lib/jquery.js", "/js/lib/jquery-foomatic.js",
+    "/js/lib/jquery.js",
-    "/js/main.js"
+    "/js/lib/jquery-foomatic.js",
+    "/js/main.js",
+    "/js/font-loader.js#with_fragment_id"
   ]}; };
 
 exports.setup = nodeunit.testCase({
@@ -39,6 +41,9 @@ exports.setup = nodeunit.testCase({
       function (err) {
         fs.writeFile('/tmp/js/main.js', "", "utf8", this);
       },
+      function (err) {
+        fs.writeFile('/tmp/js/font-loader.js', "", "utf8", this);
+      },
       function (err) {
         cb();
       }
@@ -48,13 +53,16 @@ exports.setup = nodeunit.testCase({
     step(
       fs.unlink('/tmp/js/main.js', this),
       function (err) {
-        fs.unlink('/tmp/js/lib/jquery-foomatic.js', this);
+        fs.unlink('/tmp/js/main.min.js', this);
       },
       function (err) {
         fs.unlink('/tmp/js/lib/jquery.js', this);
       },
       function (err) {
-        fs.unlink('/tmp/js/main.min.js', this);
+        fs.unlink('/tmp/js/lib/jquery-foomatic.js', this);
+      },
+      function (err) {
+        fs.unlink('/tmp/js/font-loader.js', this);
       },
       function (err) {
         fs.rmdir('/tmp/js/lib', this);
@@ -76,10 +84,12 @@ exports.setup = nodeunit.testCase({
         files,
         links,
         mddlwr;
+
     mddlwr = cachify.setup(assets, {
                              root: '/tmp',
                              production: false,
                              debug: true});
+
     links = cachify.cachify_js("/js/main.min.js").split('\n');
     test.equal(links[0], '<script src="/d41d8cd98f/js/lib/jquery.js"></script>',
               "debug option puts hash in all urls");
@@ -112,11 +122,17 @@ exports.setup = nodeunit.testCase({
                       production: false
     });
     var links = cachify.cachify_js("/js/main.min.js").split('\n');
-    test.ok(links.length, "Multiple script tags");
+    test.equal(links.length, assets["/js/main.min.js"].length,
+              "All script tags created");
     test.equal(links[0], '<script src="/js/lib/jquery.js"></script>',
               "No hashes in all urls during development");
+    test.equal(links[3], '<script src="/js/font-loader.js#with_fragment_id"></script>',
+              "Fragment identifier in URL is preserved");
+    test.equal(cachify.uncached_resources.indexOf("/js/font-loader.js#with_fragment_id"), -1,
+              "Fragment identifiers are never sent to server, search on disk for resource by removing fragment id");
     var files = cachify.cachify("/js/main.min.js").split('\n');
-    test.ok(files.length, "Multiple script tags");
+    test.equal(files.length, assets["/js/main.min.js"].length,
+              "All urls created");
     test.equal(files[0], '/js/lib/jquery.js',
               "No hashes in all urls during development");
     mddlwr(req, resp, function () {
@@ -240,4 +256,4 @@ exports.setup = nodeunit.testCase({
       test.done();
     });
   }
-});
+});