Feature Request: pass tree-relative path to `customHash`

[jamesarosen]

The documentation for customHash reads

customHash - Default: none - If set, overrides the md5 checksum calculation with the result of calling customHash(buffer). If it is not a function, customHash is used as the hash value. If it is set to null, fingerprinting is skipped and only prepending occurs.

The code actually calls it two different ways:

1. in getDestFilePath, this.hashFn(file, tmpPath), where file is the contents of the file as a string
2. in writeRailsManifest, this.hashFn(contents)

I'm trying to use broccoli-asset-rev to build a CSS library and I'd like to emit

1. my-library/index.css to my-library/index-1.2.3.css based on process.env.TAG
2. every other asset with an md5 hash

Additionally, because the md5 hash function is a closure-local, I can't get access to it. I could just copy the implementation, but I'd love if it made available, say as a third argument.

I would write something like

tree = assetRev(tree, {
  customHash: function(contents, inputPath, defaultHashFn) {
    if (process.env.TAG && path === 'my-library/index.css') {
      return process.env.TAG;
    }
    return defaultHashFn(content, inputPath);
  }
});

[jamesarosen]

I'm happy to make a PR if you think this sounds reasonable. If you have any alternative suggestions for this use-case, I'm happy to tweak the API as well.

[rickharrison]

I'm not necessarily against it, but it seems like it complicates the API a little bit, when you could just include the 3 line hashing yourself in your custom function.

if (procesnt.env.TAG ...) {

}

var md5 = crypto.createHash('md5');
md5.update(buf);
return md5.digest('hex');

[jamesarosen]

Including the hashing function as a third argument is a minor point. I don't mind losing that.

The core was that I wanted the signature of customHash to be (contents, treeRelativePath) so I could use a different function for a subset of resources. Although I suppose I could get the same effect via

indexCss = funnel(tree, { files: [ 'my-library/index.css' ] });
versionedIndexCss = assetRev(indexCss, { customHash: process.env.TAG });

everythingElse = funnel(tree, { exclude: [ 'my-library/index.css' ] });
fingerprintedElse = assetRev(everythingElse);

tree = mergeTrees([ versionedIndexCss, fingerprintedElse ]);

but that's a bit more work both for me and for the computer.

[rickharrison]

Isn't the path already being passed in?

[jamesarosen]

Sort of. It's passing in the full filesystem path rather than the tree-relative path. I don't have access to the exact tempdir prefix, so I can't easily strip it off. It's also not documented, so I worry that the feature might go away.

[rickharrison]

It's not going to go away. Can you check for something like indexOf('my-library/index.css') ?

[jamesarosen]

Yeah I can.

[rickharrison]

It's honestly a fault in the documentation. I need to do a full audit as there are a few features here and there that are permanent but still undocumented.