From 0ad1f7db7bc540e376cca123964c99d5c11cb342 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Wed, 29 Nov 2017 18:01:36 -0500 Subject: [PATCH] Decouple `config.asset_host` assignment from rails: - There is two way to set the asset_host in rails: `config.action_controller.asset_host=` and `config.asset_host=` (the latter being usefull to both set action_mailer/action_controller asset_host) - Opening this PR for two reasons: 1. Getting the asset_host from `config.action_controller` currently works when the asset_host is defined using `config.asset_host` but I feel like this might be by chance: - When rails initialization process kicks in, the `action_controller.set_configs` initializer runs. A variable `options` is assigned and reference `config.action_controller`. The `options` variable get modified and by doing so modify the refernce to `config.action_controller` https://github.com/rails/rails/blob/1bee2fb600c07625b830afd33b43ead3364c9715/actionpack/lib/action_controller/railtie.rb#L50 2. If a gem decide to set the asset_host automatically for any apps, the gem can do so inside an initializer. ```ruby initializer :set_asset_host do |app| app.config.asset_host = 'https://example.com' end ``` This code won't have any effect in sprockets-rails and as a result any assets referenced inside css, i.e. (`.div { background-image: url('/assets/rails.png') }) will not be prefixed by the asset_host --- lib/sprockets/railtie.rb | 5 +++++ test/test_railtie.rb | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/lib/sprockets/railtie.rb b/lib/sprockets/railtie.rb index 3ff11003..fc11ff1f 100644 --- a/lib/sprockets/railtie.rb +++ b/lib/sprockets/railtie.rb @@ -201,6 +201,11 @@ def self.build_manifest(app) Sprockets::Manifest.new(app.assets, path, config.assets.manifest) end + config.after_initialize do |app| + config.action_controller.asset_host ||= app.config.asset_host + config.action_controller.relative_url_root ||= app.config.relative_url_root + end + config.after_initialize do |app| config = app.config diff --git a/test/test_railtie.rb b/test/test_railtie.rb index 8e964ee5..988dfe7a 100644 --- a/test/test_railtie.rb +++ b/test/test_railtie.rb @@ -410,4 +410,18 @@ def test_quiet_assets_inserts_middleware assert middleware.include?(Sprockets::Rails::QuietAssets) assert middleware.each_cons(2).include?([Sprockets::Rails::QuietAssets, Rails::Rack::Logger]) end + + def test_config_asset_host_can_be_set_inside_an_initializer + app.configure do + initializer 'asset_host_and_relative_url_root' do |app| + app.config.asset_host = 'https://example.com' + app.config.relative_url_root = '/myassets' + end + end + app.initialize! + context_class = Sprockets::Railtie.build_environment(app).context_class + + assert_equal 'https://example.com', context_class.config.asset_host + assert_equal '/myassets', context_class.config.relative_url_root + end end