Skip to content

Commit

Permalink
Configuration changes for asset pipeline: remove config.assets.allow_…
Browse files Browse the repository at this point in the history
…debugging, add config.assets.compile and config.assets.digest
  • Loading branch information
guilleiguaran committed Aug 30, 2011
1 parent 4a60a9e commit f443f9c
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 56 deletions.
6 changes: 2 additions & 4 deletions actionpack/lib/sprockets/assets.rake
Expand Up @@ -46,10 +46,8 @@ namespace :assets do
env.precompile(*assets) env.precompile(*assets)
end end


if config.assets.manifest File.open("#{target}/manifest.yml", 'w') do |f|
File.open("#{target}/manifest.yml", 'w') do |f| YAML.dump(manifest, f)
YAML.dump(manifest, f)
end
end end
end end
end end
Expand Down
24 changes: 9 additions & 15 deletions actionpack/lib/sprockets/helpers/rails_helper.rb
Expand Up @@ -72,7 +72,7 @@ def asset_path(source, default_ext = nil, body = false, protocol = nil)


private private
def debug_assets? def debug_assets?
Rails.application.config.assets.allow_debugging && Rails.application.config.assets.compile &&
(Rails.application.config.assets.debug || (Rails.application.config.assets.debug ||
params[:debug_assets] == '1' || params[:debug_assets] == '1' ||
params[:debug_assets] == 'true') params[:debug_assets] == 'true')
Expand Down Expand Up @@ -125,22 +125,21 @@ def digest_for(logical_path)
return digest return digest
end end


if digest.nil? && Rails.application.config.assets.precompile_only if Rails.application.config.assets.compile
raise AssetNotPrecompiledError if asset = asset_environment[logical_path]
end return asset.digest_path

end
if asset = asset_environment[logical_path] return logical_path
return asset.digest_path else
raise AssetNotPrecompiledError.new("#{logical_path} isn't precompiled")

This comment has been minimized.

Copy link
@spohlenz

spohlenz Sep 2, 2011

Contributor

Thinking more about this, raising an error seems like a fairly drastic response to a missing asset. I'd prefer to simply have the asset 404 rather than have the entire page error out (especially given as this is likely to crop up only on deploying to production).

Can we replace raise AssetNotPrecompiledError with return logical_path? I've also noticed that we probably shouldn't be mounting the assets in the railtie if config.assets.compile is false.

What do you think @guilleiguaran, @spastorino?

This comment has been minimized.

Copy link
@guilleiguaran

guilleiguaran Sep 4, 2011

Author Member

cc @NZKoz

end end

logical_path
end end


def rewrite_asset_path(source, dir) def rewrite_asset_path(source, dir)
if source[0] == ?/ if source[0] == ?/
source source
else else
source = digest_for(source) if performing_caching? source = digest_for(source) if Rails.application.config.assets.digest
source = File.join(dir, source) source = File.join(dir, source)
source = "/#{source}" unless source =~ /^\// source = "/#{source}" unless source =~ /^\//
source source
Expand All @@ -154,11 +153,6 @@ def rewrite_extension(source, dir, ext)
source source
end end
end end

# When included in Sprockets::Context, we need to ask the top-level config as the controller is not available
def performing_caching?
config.action_controller.present? ? config.action_controller.perform_caching : config.perform_caching
end
end end
end end
end end
Expand Down
6 changes: 2 additions & 4 deletions actionpack/lib/sprockets/railtie.rb
Expand Up @@ -26,10 +26,8 @@ class Railtie < ::Rails::Railtie
end end
end end


if config.assets.manifest if File.exist?(path = File.join(Rails.public_path, config.assets.prefix, "manifest.yml"))
if File.exist?(path = File.join(Rails.public_path, config.assets.prefix, "manifest.yml")) config.assets.digests = YAML.load_file(path)
config.assets.digests = YAML.load_file(path)
end
end end


ActiveSupport.on_load(:action_view) do ActiveSupport.on_load(:action_view) do
Expand Down
6 changes: 4 additions & 2 deletions actionpack/test/template/sprockets_helper_test.rb
Expand Up @@ -33,6 +33,8 @@ def host_with_port() 'localhost' end
@config = config @config = config
@config.action_controller ||= ActiveSupport::InheritableOptions.new @config.action_controller ||= ActiveSupport::InheritableOptions.new
@config.perform_caching = true @config.perform_caching = true
@config.assets.digest = true
@config.assets.compile = true
end end


def url_for(*args) def url_for(*args)
Expand Down Expand Up @@ -160,7 +162,7 @@ def url_for(*args)
assert_match %r{<script src="/assets/xmlhr-[0-9a-f]+.js\?body=1" type="text/javascript"></script>\n<script src="/assets/application-[0-9a-f]+.js\?body=1" type="text/javascript"></script>}, assert_match %r{<script src="/assets/xmlhr-[0-9a-f]+.js\?body=1" type="text/javascript"></script>\n<script src="/assets/application-[0-9a-f]+.js\?body=1" type="text/javascript"></script>},
javascript_include_tag(:application, :debug => true) javascript_include_tag(:application, :debug => true)


@config.assets.allow_debugging = true @config.assets.compile = true
@config.assets.debug = true @config.assets.debug = true
assert_match %r{<script src="/assets/xmlhr-[0-9a-f]+.js\?body=1" type="text/javascript"></script>\n<script src="/assets/application-[0-9a-f]+.js\?body=1" type="text/javascript"></script>}, assert_match %r{<script src="/assets/xmlhr-[0-9a-f]+.js\?body=1" type="text/javascript"></script>\n<script src="/assets/application-[0-9a-f]+.js\?body=1" type="text/javascript"></script>},
javascript_include_tag(:application) javascript_include_tag(:application)
Expand Down Expand Up @@ -201,7 +203,7 @@ def url_for(*args)
assert_match %r{<link href="/assets/style-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />\n<link href="/assets/application-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />}, assert_match %r{<link href="/assets/style-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />\n<link href="/assets/application-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />},
stylesheet_link_tag(:application, :debug => true) stylesheet_link_tag(:application, :debug => true)


@config.assets.allow_debugging = true @config.assets.compile = true
@config.assets.debug = true @config.assets.debug = true
assert_match %r{<link href="/assets/style-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />\n<link href="/assets/application-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />}, assert_match %r{<link href="/assets/style-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />\n<link href="/assets/application-[0-9a-f]+.css\?body=1" media="screen" rel="stylesheet" type="text/css" />},
stylesheet_link_tag(:application) stylesheet_link_tag(:application)
Expand Down
5 changes: 2 additions & 3 deletions railties/lib/rails/application/configuration.rb
Expand Up @@ -39,9 +39,8 @@ def initialize(*)
@assets.prefix = "/assets" @assets.prefix = "/assets"
@assets.version = '' @assets.version = ''
@assets.debug = false @assets.debug = false
@assets.allow_debugging = false @assets.compile = true
@assets.manifest = true @assets.digest = false
@assets.precompile_only = false
@assets.cache_store = [ :file_store, "#{root}/tmp/cache/assets/" ] @assets.cache_store = [ :file_store, "#{root}/tmp/cache/assets/" ]
@assets.js_compressor = nil @assets.js_compressor = nil
@assets.css_compressor = nil @assets.css_compressor = nil
Expand Down
Expand Up @@ -52,10 +52,6 @@ class Application < Rails::Application
<% unless options.skip_sprockets? -%> <% unless options.skip_sprockets? -%>
# Enable the asset pipeline # Enable the asset pipeline
config.assets.enabled = true config.assets.enabled = true

# Create a manifest with the hashes of your assets when you run "rake assets:precompile".
# Use this if you don't have a JavaScript engine in your production servers
config.assets.manifest = true
<% end -%> <% end -%>
end end
end end
Expand Up @@ -25,9 +25,6 @@
# Do not compress assets # Do not compress assets
config.assets.compress = false config.assets.compress = false


# Allow pass debug_assets=true as a query parameter to load pages with unpackaged assets
config.assets.allow_debugging = true

# Expands the lines which load the assets # Expands the lines which load the assets
config.assets.debug = true config.assets.debug = true
end end
Expand Up @@ -14,6 +14,12 @@
# Compress JavaScripts and CSS # Compress JavaScripts and CSS
config.assets.compress = true config.assets.compress = true


# Don't fallback to assets pipeline if a precompiled asset is missed
config.assets.compile = false

# Generate digests for assets URLs
config.assets.digest = true

# Specifies the header that your server uses for sending files # Specifies the header that your server uses for sending files
# config.action_dispatch.x_sendfile_header = "X-Sendfile" # for apache # config.action_dispatch.x_sendfile_header = "X-Sendfile" # for apache
# config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for nginx # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for nginx
Expand Down
45 changes: 24 additions & 21 deletions railties/test/application/assets_test.rb
Expand Up @@ -37,6 +37,8 @@ def app


test "assets do not require compressors until it is used" do test "assets do not require compressors until it is used" do
app_file "app/assets/javascripts/demo.js.erb", "<%= :alert %>();" app_file "app/assets/javascripts/demo.js.erb", "<%= :alert %>();"
app_file "config/initializers/compile.rb", "Rails.application.config.assets.compile = true"

ENV["RAILS_ENV"] = "production" ENV["RAILS_ENV"] = "production"
require "#{app_path}/config/environment" require "#{app_path}/config/environment"


Expand All @@ -62,18 +64,7 @@ def app
end end
end end


test "precompile don't create a manifest file when manifest option is off" do test "precompile creates a manifest file with all the assets listed" do
app_file "app/assets/javascripts/application.js", "alert();"
app_file "config/initializers/manifest.rb", "Rails.application.config.assets.manifest = false"

capture(:stdout) do
Dir.chdir(app_path){ `bundle exec rake assets:precompile` }
end

assert !File.exist?("#{app_path}/public/assets/manifest.yml")
end

test "precompile creates a manifest file with all the assets listed when manifest option is on" do
app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>"
app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/application.js", "alert();"


Expand All @@ -88,7 +79,7 @@ def app
assert_match /application-([0-z]+)\.css/, assets["application.css"] assert_match /application-([0-z]+)\.css/, assets["application.css"]
end end


test "assets do not require any assets group gem when manifest option is on and manifest file is present" do test "assets do not require any assets group gem when manifest file is present" do
app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/application.js", "alert();"


ENV["RAILS_ENV"] = "production" ENV["RAILS_ENV"] = "production"
Expand All @@ -108,10 +99,8 @@ def app
assert !defined?(Uglifier) assert !defined?(Uglifier)
end end


test "assets raise AssetNotPrecompiledError if config.assets.precompile_only is on and file isn't precompiled" do test "assets raise AssetNotPrecompiledError when manifest file is present and requested file isn't precompiled" do
app_file "app/assets/javascripts/app.js", "alert();"
app_file "app/views/posts/index.html.erb", "<%= javascript_include_tag 'app' %>" app_file "app/views/posts/index.html.erb", "<%= javascript_include_tag 'app' %>"
app_file "config/initializers/precompile_only.rb", "Rails.application.config.assets.precompile_only = true"


app_file "config/routes.rb", <<-RUBY app_file "config/routes.rb", <<-RUBY
AppTemplate::Application.routes.draw do AppTemplate::Application.routes.draw do
Expand All @@ -120,15 +109,25 @@ def app
RUBY RUBY


ENV["RAILS_ENV"] = "production" ENV["RAILS_ENV"] = "production"
capture(:stdout) do
Dir.chdir(app_path){ `bundle exec rake assets:precompile` }
end

# Create file after of precompile
app_file "app/assets/javascripts/app.js", "alert();"

require "#{app_path}/config/environment" require "#{app_path}/config/environment"
class ::PostsController < ActionController::Base ; end class ::PostsController < ActionController::Base ; end


get '/posts' get '/posts'
assert_match /AssetNotPrecompiledError/, last_response.body assert_match /AssetNotPrecompiledError/, last_response.body
assert_match /app.js isn't precompiled/, last_response.body
end end


test "precompile appends the md5 hash to files referenced with asset_path and run in the provided RAILS_ENV" do test "precompile appends the md5 hash to files referenced with asset_path and run in the provided RAILS_ENV" do
app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>"
# digest is default in false, we must enable it for test environment
app_file "config/initializers/compile.rb", "Rails.application.config.assets.digest = true"


# capture(:stdout) do # capture(:stdout) do
Dir.chdir(app_path){ `bundle exec rake assets:precompile RAILS_ENV=test` } Dir.chdir(app_path){ `bundle exec rake assets:precompile RAILS_ENV=test` }
Expand All @@ -139,6 +138,7 @@ class ::PostsController < ActionController::Base ; end


test "precompile appends the md5 hash to files referenced with asset_path and run in production as default even using RAILS_GROUPS=assets" do test "precompile appends the md5 hash to files referenced with asset_path and run in production as default even using RAILS_GROUPS=assets" do
app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>"
app_file "config/initializers/compile.rb", "Rails.application.config.assets.compile = true"


ENV["RAILS_ENV"] = nil ENV["RAILS_ENV"] = nil
capture(:stdout) do capture(:stdout) do
Expand Down Expand Up @@ -201,24 +201,27 @@ def index
assert_equal 200, last_response.status assert_equal 200, last_response.status
end end


test "assets are concatenated when debug is off and allow_debugging is off either if debug_assets param is provided" do test "assets are concatenated when debug is off and compile is off either if debug_assets param is provided" do
app_with_assets_in_view app_with_assets_in_view


# config.assets.debug and config.assets.allow_debugging are false for production environment # config.assets.debug and config.assets.compile are false for production environment
ENV["RAILS_ENV"] = "production" ENV["RAILS_ENV"] = "production"
capture(:stdout) do
Dir.chdir(app_path){ `bundle exec rake assets:precompile` }
end
require "#{app_path}/config/environment" require "#{app_path}/config/environment"


class ::PostsController < ActionController::Base ; end class ::PostsController < ActionController::Base ; end


# the debug_assets params isn't used if allow_debugging is off # the debug_assets params isn't used if compile is off
get '/posts?debug_assets=true' get '/posts?debug_assets=true'
assert_match /<script src="\/assets\/application-([0-z]+)\.js" type="text\/javascript"><\/script>/, last_response.body assert_match /<script src="\/assets\/application-([0-z]+)\.js" type="text\/javascript"><\/script>/, last_response.body
assert_no_match /<script src="\/assets\/xmlhr-([0-z]+)\.js" type="text\/javascript"><\/script>/, last_response.body assert_no_match /<script src="\/assets\/xmlhr-([0-z]+)\.js" type="text\/javascript"><\/script>/, last_response.body
end end


test "assets aren't concatened when allow_debugging is on and debug_assets params is true" do test "assets aren't concatened when compile is true is on and debug_assets params is true" do
app_with_assets_in_view app_with_assets_in_view
app_file "config/initializers/allow_debugging.rb", "Rails.application.config.assets.allow_debugging = true" app_file "config/initializers/compile.rb", "Rails.application.config.assets.compile = true"


ENV["RAILS_ENV"] = "production" ENV["RAILS_ENV"] = "production"
require "#{app_path}/config/environment" require "#{app_path}/config/environment"
Expand Down

3 comments on commit f443f9c

@spohlenz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assets:precompile rake task will need to be modified for the case where config.assets.digest is set to false in production.

I am also seeing some unexpected "X isn't precompiled" errors when config.assets.compile is set to false. Not sure whether there's an issue there or not -- I will investigate further tomorrow.

@spohlenz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I'm not the only person experiencing "X isn't precompiled" errors: #2763.
Perhaps a solution is to explicitly set config.assets.compile to true within the rake task.

@guilleiguaran
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spohlenz: Yes, looks like an issue when config.assets.digest and config.assets.compile both are false.

Please sign in to comment.