bminer / node-static-asset

Static asset manager for Node.JS and Express
MIT License
48 stars 13 forks source link

Export assetFingerprint method on middleware function #8

Closed seanami closed 10 years ago

seanami commented 10 years ago

I found myself trying to integrate this minimal static asset fingerprinting library (which I love, by the way) with the Less pre-processor for CSS and ran into problems because the assetFingerprint method is only available from the request and locals within templates. In writing a custom Less function to wrap assetFingerprint, I don't have access to either the request object or the template locals, so I needed a reference to the method another way. This is the simplest change that I could come up with that made things workable.

With this change, I can do something like the following when setting up my middleware:

// 1. Create the static asset middleware and the private in-memory fingerprint cache
var staticAssetMiddleware = staticAsset(assetsPath);

// 2. The assetFingerprint method is exported on the middleware function
var assetFingerprint = staticAssetMiddleware.assetFingerprint;

// 3. Set up other pre-processor middleware here that needs to fingerprint assets here
app.use(...);

// 4. Once the pre-processors have run and the resulting CSS is on the disk, insert the static asset middleware
app.use(staticAssetMiddleware);

In my case, my Less middleware setup for step 3 looked like this, for reference:

app.use(lessMiddleware({
  src: path.join(__dirname, 'public'),
  once: app.get('env') !== 'development',
  compress: app.get('env') !== 'development',
  treeFunctions: {
    // Make assetFingerprint available within less stylesheets
    asseturl: function(str) {
      var assetPath = url.resolve('/stylesheets/', str.value);
      var urlString = '"' + staticAssetMiddleware.assetFingerprint(assetPath) + '"';
      return new(less.tree.URL)(new(less.tree.Anonymous)(urlString)); //.eval(this.env);
    },
  },
}));

If you have a better suggestion on how to achieve this instead, let me know.

bminer commented 10 years ago

@seanami - I have no problem merging this change. Have you tested this PR?

seanami commented 10 years ago

I didn't see any tests in this repo that I could run, but my branch is deployed as part of my own project and things seem to be working the same as before my change.

bminer commented 10 years ago

@seanami - Good enough for me. Just wanted to make sure you deployed exactly what was in the PR. :) I will merge. Do you need me to bump the version number and publish to NPM?

seanami commented 10 years ago

Thanks! If you have time to bump the version and publish, that would be cool, since I could go back to using a vanilla NPM version of the library. But I've already got my branch out, so there's no rush from me other than cleanliness.

bminer commented 10 years ago

Published to NPM. All set! Thanks again for the PR!