Permalink
Browse files

config.action_controller.perform_caching isn't used anymore in assets…

… pipeline, instead we are using config.assets.digest now
  • Loading branch information...
1 parent 6521cf3 commit 6fc518e2ec59ec00076aaca08b9e3df3baee54a3 @guilleiguaran guilleiguaran committed Sep 6, 2011
Showing with 10 additions and 1 deletion.
  1. +1 −1 actionpack/lib/sprockets/railtie.rb
  2. +9 −0 railties/test/application/assets_test.rb
@@ -71,7 +71,7 @@ class Railtie < ::Rails::Railtie
mount app.assets => config.assets.prefix
end
- if config.action_controller.perform_caching
+ if config.assets.digest
app.assets = app.assets.index
end
end
@@ -64,6 +64,15 @@ def app
end
end
+ test "asset pipeline should use a Sprockets::Index when config.assets.digest is true" do
+ app_file "config/initializers/digest.rb", "Rails.application.config.assets.digest = true"
+ app_file "config/initializers/caching.rb", "Rails.application.config.action_controller.perform_caching = false"
+ ENV["RAILS_ENV"] = "production"
+ require "#{app_path}/config/environment"
+
+ assert_equal Sprockets::Index, Rails.application.assets.class
+ end
+
test "precompile creates a manifest file with all the assets listed" do
app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>"
app_file "app/assets/javascripts/application.js", "alert();"

10 comments on commit 6fc518e

@spohlenz

I think action_controller.perform_caching was correct in this situation.

The Sprockets index is related to caching, and doesn't really have anything to do with digests.

@guilleiguaran
Member

@spohlenz, we added the config.assets.digest just to remove all the references of config.action_controller.perform_caching in asset pipeline:

f443f9c#L1R142
f443f9c#L1L159

@spohlenz

I'm not sure if removing it entirely is all that necessary IMHO.

The other changes were reasonable because the perform_caching flag was used to control whether digests were added or not. However they're two separate functions and the appropriate configuration variables should be used for each.

@josh
Member
josh commented on 6fc518e Sep 7, 2011

perform_caching still makes sense here

@josevalim
Member

We should stop using perform_caching for everything. We should have decoupled options. Those decouples options could have their default value depending on perform_caching, but we do need to have decoupled options.

@josevalim
Member

We should stop using perform_caching for everything. We should have decoupled options. Those decouples options could have their default value depending on perform_caching, but we do need to have decoupled options.

@josh
Member
josh commented on 6fc518e Sep 7, 2011

Then you need to add another option for config.assets.caching.

Enabling sprockets production caching isn't the same thing as flipping digests off and on.

@josevalim
Member

Sounds good.

@jeremy
Member
jeremy commented on 6fc518e Oct 7, 2012

Ancient history now, but 👍 to config.assets.caching to cache the environment (or the reverse, config.assets.reload ?)

@guilleiguaran
Member

Right, the new option was never added and I think this is causing #6803

Please sign in to comment.