Synthesis

Great work you’ve done on the plugin, but I’m not sure I like the idea of the “current version.”

I’d much prefer to keep this out of the plugin and just have it deal with merging. You could then let rails use the default asset timestamping or your own that’s based on individual (as opposed to app-wide) asset revisions.

I don’t think you need to worry too much about the whole “not all browsers will cache javascript files with query string parameters.”

Actually I don’t mind your timestamp-on-URL approach, but more intelligent timestamping would be better as each new deployment would require a redownload of all assets, even if the file hasn’t itself changed.

and in case anybody wants to rip it apart and improve/modify it, what license is it released under?

It’s released under the MIT license. I’ll add that text to the Readme file. Feel free to rip it up! It’d be great if you could send any improvements you make back my way so I could include them.

I’m going to add a few things more things I’ve thought of to improve it…

This is great work – aiming to create what Flickr uses in their deployments but making it easier for rails.

I think the next biggest gains would come in merging and compressing the stylesheets as well, as Tim mentioned in my entry’s comment. I know there’s quite a few CSS compressors on the web, I wonder if there’s any we could port (or if someone’s already done this).

I agree with Tim that the timestamping via query params isn’t such a huige issue becaise Firefox and IE don’t actually follow spec and it’ll work for them, but very high traffic sites or sites where the average users come from Mac owners or mobile phones should definitely be aware of the issues.

Yes, I have an eye towards making this work for CSS assets too. And probably change the name to something like MergeAssets. :)

As far as the timestamps issue, I like having one solution that works for everyone. Why ignore the others?

Great Plugin

However, to get this working on Windows, I had to edit the line in the file that executes ruby to include quote marks around the file paths (including the one to jsmin.rb). I also added spaces between the < and >. Could this get fixed in the main version?

(The problem is due to the fact if your Rails Root has a folder with a space in the name (like “Documents and Settings”) it gets confused.)

Thats funny since it was working fine on windows for me. (No names with spaces, thats probably why.) I’ll see about adding that, as long as it doesn’t break it on other platforms.

I too wasn’t a big fan of the timestamps, so I went ahead and reworked the source a little to look for the highest subversion revision number of any of the scripts in a specific package. The config can than look more like this:

--- 
source_map: 
- base_121: 
  - prototype
  - effects
  - event-selectors
- search_60
  -search

This way the caching will occur per package, seems to be working well for me. Could definatly be made more robust. I’ll go ahead and submit my changes to Scott

For the CSS cleanups you might want to look at http://www.issuetrackerproduct.com/Download/#slimmer Slimmer. I’m sure a conversion of the CSS porition of it to Ruby would be trivial.

I love this plugin. I wrote my own merge script, but this one squeeze a few more kilobytes out of my scripts.

The timestamped filename also works better than Rails 1.1 query string timestamps. Scripts that use query strings are not cached by Safari, so you lose a bit. Still, I don’t see a lot of Safari traffic so I decided to go with query strings.

I do have one recommendation. In the rake task, check if the javascripts directory contains .svn. If it does, do svn delete/add for each script, that way it only takes one svn commit to upload the merged script to the repository.

Another worthwhile enhancement may be to modify the asset helpers to allow for “shorthand” that loads multiple .js/.css files given the name/symbol of their associated parent, merged file as defined in the YAML. An example is shown below.

This should format better…

Given the following snippet from the YAML file:

javascripts:
- base:
  - prototype
  - effects
  - controls
  - dragdrop
  - application
- secondary:
  - foo
  - bar

<%= javascript_include_merged :base %>
would result in the following markup being generated in non-merging environments:

  <script type="text/javascript" src="/javascripts/prototype.js"></script>
  <script type="text/javascript" src="/javascripts/effects.js"></script>
  <script type="text/javascript" src="/javascripts/controls.js"></script>
  <script type="text/javascript" src="/javascripts/dragdrop.js"></script>
  <script type="text/javascript" src="/javascripts/application.js"></script>

It would follow that
<%= javascript_include_merged :secondary %>
would result in the following markup being generated in non-merging environments:

<script type="text/javascript" src="/javascripts/foo.js"></script>
<script type="text/javascript" src="/javascripts/bar.js"></script>

That type of shorthand would be preferable to the current method of specifying the files separately as follows:

<%= javascript_include_merged 'foo', 'bar' %>