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

Deprecate asset fallback #375

Merged
merged 8 commits into from
Sep 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,12 @@ Each asset task will invoke `assets:environment` first. By default this loads th

Also see [Sprockets::Rails::Task](https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/task.rb) and [Rake::SprocketsTask](https://github.com/rails/sprockets/blob/master/lib/rake/sprocketstask.rb).


### Initializer options

**`config.assets.unknown_asset_fallback`**

When set to a truthy value, a result will be returned even if the requested asset is not found in the asset pipeline. When set to a falsey value it will raise an error when no asset is found in the pipeline. Defaults to `true`.

**`config.assets.precompile`**

Add additional assets to compile on deploy. Defaults to `application.js`, `application.css` and any other non-js/css file under `app/assets`.
Expand Down
19 changes: 18 additions & 1 deletion lib/sprockets/rails/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
module Sprockets
module Rails
module Helper
class AssetNotFound < StandardError; end
Copy link

Choose a reason for hiding this comment

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

Shouldn't we add a newline after this class definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

done


class AssetNotPrecompiled < StandardError
include Sprockets::Rails::Utils
def initialize(source)
Expand Down Expand Up @@ -33,7 +35,8 @@ def initialize(source)
:assets_environment, :assets_manifest,
:assets_precompile, :precompiled_asset_checker,
:assets_prefix, :digest_assets, :debug_assets,
:resolve_assets_with, :check_precompiled_asset
:resolve_assets_with, :check_precompiled_asset,
:unknown_asset_fallback
]

def self.included(klass)
Expand Down Expand Up @@ -68,12 +71,26 @@ def assets_environment
end
end

# Writes over the built in ActionView::Helpers::AssetUrlHelper#compute_asset_path
# to use the asset pipeline.
def compute_asset_path(path, options = {})
debug = options[:debug]

if asset_path = resolve_asset_path(path, debug)
File.join(assets_prefix || "/", legacy_debug_path(asset_path, debug))
else
message = "The asset #{ path.inspect } is not present in the asset pipeline."
raise AssetNotFound, message unless unknown_asset_fallback

if respond_to?(:public_compute_asset_path)
message << "Falling back to an asset that may be in the public folder.\n"
message << "This behavior is deprecated and will be removed.\n"
message << "To bypass the asset pipeline and preserve this behavior,\n"
message << "use the `skip_pipeline: true` option.\n"

call_stack = respond_to?(:caller_locations) ? caller_locations : caller
Copy link
Member

Choose a reason for hiding this comment

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

If we're on a Rails new enough to have public_compute_asset_path, that implies we're on a Ruby new enough to have caller_locations, doesn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The tests don't actually use Rails 5.1, we define the method, and the tests get run on all Ruby versions. The alternative to this would be detecting ruby versions and skipping the tests, which is less clean. I also want to be able to make a gem so that older versions of Rails can use this flag to get the error/deprecation behavior.

ActiveSupport::Deprecation.warn(message, call_stack)
end
super
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/sprockets/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ def configure(&block)
config.assets.cache_limit = 50.megabytes
config.assets.gzip = true
config.assets.check_precompiled_asset = true
config.assets.unknown_asset_fallback = true

config.assets.configure do |env|
config.assets.paths.each { |path| env.append_path(path) }
Expand Down Expand Up @@ -245,7 +246,7 @@ def self.build_manifest(app)
self.resolve_assets_with = config.assets.resolve_with

self.check_precompiled_asset = config.assets.check_precompiled_asset

self.unknown_asset_fallback = config.assets.unknown_asset_fallback
# Expose the app precompiled asset check to the view
self.precompiled_asset_checker = -> logical_path { app.asset_precompiled? logical_path }
end
Expand Down
32 changes: 32 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def setup
@view.assets_precompile = %w( manifest.js )
precompiled_assets = @manifest.find(@view.assets_precompile).map(&:logical_path)
@view.check_precompiled_asset = true
@view.unknown_asset_fallback = true
@view.precompiled_asset_checker = -> logical_path { precompiled_assets.include? logical_path }
@view.request = ActionDispatch::Request.new({
"rack.url_scheme" => "https"
Expand Down Expand Up @@ -827,6 +828,37 @@ def test_index_files
end
end

class DeprecationTest < HelperTest
def test_deprecations_for_asset_path
@view.send(:define_singleton_method, :public_compute_asset_path, -> {})
assert_deprecated do
@view.asset_path("does_not_exist.noextension")
end
ensure
@view.instance_eval('undef :public_compute_asset_path')
end

def test_deprecations_for_asset_url
@view.send(:define_singleton_method, :public_compute_asset_path, -> {})

assert_deprecated do
@view.asset_url("does_not_exist.noextension")
end
ensure
@view.instance_eval('undef :public_compute_asset_path')
end

def test_deprecations_for_image_tag
@view.send(:define_singleton_method, :public_compute_asset_path, -> {})

assert_deprecated do
@view.image_tag("does_not_exist.noextension")
end
ensure
@view.instance_eval('undef :public_compute_asset_path')
end
end

class RaiseUnlessPrecompiledAssetDisabledTest < HelperTest
def test_check_precompiled_asset_enabled
@view.check_precompiled_asset = true
Expand Down