Skip to content
Browse files

Asset manifest includes aliases for foo.js -> foo/index.js and vice v…

…ersa. Bump Sprockets requirements from 2.1+ to 2.2+ and let it answer "should we compile this asset?" for us.
  • Loading branch information...
1 parent 5cb5092 commit 19987b64af0d543a6292c3fd9b0b877f70eab306 @jeremy jeremy committed Sep 30, 2012
View
14 actionpack/CHANGELOG.md
@@ -1,5 +1,19 @@
## Rails 3.2.9 (unreleased) ##
+* Precompiled assets include aliases from foo.js to foo/index.js and vice versa.
+
+ # Precompiles phone-<digest>.css and aliases phone/index.css to phone.css.
+ config.assets.precompile = [ 'phone.css' ]
+
+ # Precompiles phone/index-<digest>.css and aliases phone.css to phone/index.css.
+ config.assets.precompile = [ 'phone/index.css' ]
+
+ # Both of these work with either precompile thanks to their aliases.
+ <%= stylesheet_link_tag 'phone', media: 'all' %>
+ <%= stylesheet_link_tag 'phone/index', media: 'all' %>
+
+ *Jeremy Kemper*
+
* `assert_template` is no more passing with what ever string that matches
with the template name.
View
2 actionpack/actionpack.gemspec
@@ -23,7 +23,7 @@ Gem::Specification.new do |s|
s.add_dependency('rack', '~> 1.4.0')
s.add_dependency('rack-test', '~> 0.6.1')
s.add_dependency('journey', '~> 1.0.4')
- s.add_dependency('sprockets', '~> 2.1')
+ s.add_dependency('sprockets', '~> 2.2')
s.add_dependency('erubis', '~> 2.7.0')
s.add_development_dependency('tzinfo', '~> 0.3.29')
View
32 actionpack/lib/sprockets/static_compiler.rb
@@ -15,13 +15,11 @@ def initialize(env, target, paths, options = {})
def compile
manifest = {}
- env.each_logical_path do |logical_path|
- if File.basename(logical_path)[/[^\.]+/, 0] == 'index'
- logical_path.sub!(/\/index\./, '.')
- end
- next unless compile_path?(logical_path)
+ env.each_logical_path(paths) do |logical_path|
if asset = env.find_asset(logical_path)
- manifest[logical_path] = write_asset(asset)
+ digest_path = write_asset(asset)
+ manifest[asset.logical_path] = digest_path
+ manifest[aliased_path_for(asset.logical_path)] = digest_path
@ndbroadbent
ndbroadbent added a note Oct 6, 2012

Sorry, should have added my comment here as a line note: 19987b6#commitcomment-1956232

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
end
write_manifest(manifest) if @manifest
@@ -43,22 +41,16 @@ def write_asset(asset)
end
end
- def compile_path?(logical_path)
- paths.each do |path|
- case path
- when Regexp
- return true if path.match(logical_path)
- when Proc
- return true if path.call(logical_path)
- else
- return true if File.fnmatch(path.to_s, logical_path)
- end
- end
- false
- end
-
def path_for(asset)
@digest ? asset.digest_path : asset.logical_path
end
+
+ def aliased_path_for(logical_path)
+ if File.basename(logical_path).start_with?('index')
+ logical_path.sub(/\/index([^\/]+)$/, '\1')
+ else
+ logical_path.sub(/\.([^\/]+)$/, '/index.\1')
+ end
+ end
end
end
View
17 railties/test/application/assets_test.rb
@@ -122,8 +122,23 @@ def assert_no_file_exists(filename)
app_file "app/assets/javascripts/something/index.js.erb", "alert();"
precompile!
-
assert File.exists?("#{app_path}/public/assets/something.js")
+
+ assets = YAML.load_file("#{app_path}/public/assets/manifest.yml")
+ assert_not_nil assets['something/index.js'], "Expected something/index.js among #{assets.keys.inspect}"
+ assert_not_nil assets['something.js'], "Expected something.js among #{assets.keys.inspect}"
+ end
+
+ test "precompile something/index.js for directory containing index file" do
+ add_to_config "config.assets.precompile = [ 'something/index.js' ]"
+ app_file "app/assets/javascripts/something/index.js.erb", "alert();"
+
+ precompile!
+ assert File.exists?("#{app_path}/public/assets/something/index.js")
+
+ assets = YAML.load_file("#{app_path}/public/assets/manifest.yml")
+ assert_not_nil assets['something/index.js'], "Expected something/index.js among #{assets.keys.inspect}"
+ assert_not_nil assets['something.js'], "Expected something.js among #{assets.keys.inspect}"
end
test "asset pipeline should use a Sprockets::Index when config.assets.digest is true" do

7 comments on commit 19987b6

@Mange
Mange commented on 19987b6 Oct 2, 2012

Thanks a lot!

@rwz
Ruby on Rails member
rwz commented on 19987b6 Oct 2, 2012

I can't believe this was so damn easy. I really tried looking into it before and it seemed much more complicated back then :)

@ndbroadbent

This commit is adding /index.* aliases for every asset in my manifest.yml, including images. I don't think this is necessary, so it can be limited by checking if the index file exists with something like:

aliased_path = aliased_path_for asset.logical_path
if File.exists? "#{target}/#{aliased_path}"
  manifest[aliased_path] = digest_path
end
@jeremy
Ruby on Rails member
jeremy commented on 19987b6 Oct 7, 2012

Sounds good @ndbroadbent - giving it a shot?

Could move the file existence check into aliased_path_for:

    def aliased_path_for(logical_path)
      if File.basename(logical_path).start_with?('index')
        logical_path.sub(/\/index([^\/]+)$/, '\1')
      else
        aliased_path = logical_path.sub(/\.([^\/]+)$/, '/index.\1')
        aliased_path if File.exist?("#{target}/#{aliased_path}")
      end
    end

Then skip absent aliases:

          if aliased_path = aliased_path_for(asset.logical_path)
            manifest[aliased_path] = digest_path
          end
@jeremy
Ruby on Rails member
jeremy commented on 19987b6 Oct 7, 2012

@rwz Simple result... of a long evening full of facepalms :)

I tried a few different approaches, but our hands are tied by asset digests which leave us without a useful sprockets environment. So the only avenue left is to use the manifest. And sprockets itself doesn't normalize logical paths for us, or even express a preference for one form over another -- foo.js and foo/index.js are peers in its eyes, yet #each_logical_path yields foo/index.js. Argh!

@ndbroadbent

@jeremy, that looks great, thanks!

While we're on the topic of assets in Rails 3.2, I was wondering if you might be interested in taking a look at my pull request to improve asset compilation: #7866

Please sign in to comment.
Something went wrong with that request. Please try again.