Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Jammit to be extended to package and compress other types of assests #229

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dinedal
Copy link

@dinedal dinedal commented Apr 24, 2012

I use Jammit on a production level service and am happy with it, however I came across a need to be able to package and compress other static assets that are not stylesheets, javascripts, or JSTs.

In order to do this, the attached changes are needed to be able to allow me to extend Jammit with custom asset types. It is important to me to keep using Jammit, and I would greatly appreciate this functionality being merged into the main repo, and I'm open to any changes you feel are needed to get this in.

Below is a modified version of what I am using to interact with the fork of Jammit with these included changes.

module Jammit

  JSON_NAMESPACE = "window.DATA"

  JSON_FUNCTION = "JSON.parse"

  JSON_EXTENSION = "json"

  class Compressor
    # no actual compression applied, just formats it correctly, but could support compression
    def compress_json(paths)
      namespace   = JSON_NAMESPACE
      base_path   = find_base_path(paths)
      compiled    = paths.map do |path|
        contents  = read_binary_file(path)
        name      = json_name(path, base_path)
        "#{namespace}['#{name}'] = #{JSON_FUNCTION}('#{contents}');"
      end
      setup_namespace = "#{namespace} = #{namespace} || {};"
      [JST_START, setup_namespace, compiled, JST_END].flatten.join("\n")
    end

    def json_name(path, base_path)
      return File.basename(path, ".#{JSON_EXTENSION}") unless base_path
      path.gsub(/\A#{Regexp.escape(base_path)}\/(.*)\.#{JSON_EXTENSION}\Z/, '\1')
    end
  end

  class Packager
    def pack_jsons(package)
      compressor.compress_json(package_for(package, :json)[:paths])
    end
  end

  module Helper
    def include_jsons(*packages)
      options = packages.extract_options!
      html_safe packages.map {|pack|
        Jammit.asset_url(pack, :json)
      }.flatten.map {|pack|
        javascript_include_tag pack, options
      }.join("\n")
    end
  end
end

Please note that this is a initial pass and is very conservative in what it changes, however a couple of more drastic changes could make this functionality more streamlined with the rest of the project.

Ideally, I feel that JSTs should also be broken out ouf the Javascript packaging / compression, and the pipeline of packaging and compressing be made more generic to support handling user defined asset types as a first class citizen. Thus, each type, user defined or included, will flow through a similar work flow.

However, before embarking on this, I would greatly appreciate feedback on what I have so far, how to determine what asset type (JS or CSS) an asset supplied in the assets.yml would be, and if the way user defined templates are created is acceptable.

@jashkenas
Copy link
Member

I'm a bit confused as to why you don't simply include these files as regular JavaScript, not JSON. That's what you're ultimately doing, isn't it?

@braddunbar
Copy link

👍 - This would allow precompiling JSTs as well, a big win for debugging since you could get a file name and line number from template errors. It's currently difficult to alter the behavior of JST compilation since it's tied into javascript compilation.

@dinedal
Copy link
Author

dinedal commented Apr 24, 2012

@jashkenas Thanks for your quick reply.

While you're correct, the example above does basically that (and is actually how we handle this case at present), however, it is more of a proof of concept then what is going to be implemented for JSONs.

JSONs have different attributes then javascripts or JSTs, they need to be stored without javascript inside of them for portability, they don't have any args that are passed to them for rendering, as they are not rendered, they can also "stack" on each other, ie, two JSONs could exist, one of default values and one including overrides of the original, and keeping them in the JST namespace means keeping code to determine if the template is a JST or a JSON in the client side. It would be cleaner to support them as a first class type.

@flippyhead
Copy link

@jashkenas hey! Now, 3 months later, any different feelings about this? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants