From 88ee52f9d9cf2068bb400205e5a0c1d2e40169d1 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Wed, 18 Dec 2019 17:23:50 +0100 Subject: [PATCH] Don't require "action_view/base" in action pack: - ### Problem ActionPack requires "action_view/base" at boot time, this causes a variety of issue that I described in detail in #38024. There is no real reason to require av/base in the ActionDispatch::Debugexceptions class. ### Solution Like any other components (such as ActiveRecord, ActiveJob...), ActionView::Base shouldn't be loaded at boot time. Here are the two main changes needed for this: 1) Actionview has a special initializer that needs to run before the app is fully booted (adding a executor needs to be done before application is done booting) https://github.com/rails/rails/blob/63ec70e700e321b22e9baf2ad2d45cd3f4febc79/actionview/lib/action_view/railtie.rb#L81-L84 That initializer used a lazy load hooks but we can't do that anymore because Action::Base view won't be triggered during booting process. When it will get triggered, (presumably on the first request), it's too late to add an executor. ------------------------------------------------ 2) Compare to other components, ActionView doesn't use `Base` for configuration flag. A lot of flags ares instead set on modules (FormHelper, FormTagHelper). The problem is that those module depends on AV::Base to be loaded, as otherwise configuration set by the user aren't applied. (Since the lazy load hooks hasn't been triggered) https://github.com/rails/rails/blob/63ec70e700e321b22e9baf2ad2d45cd3f4febc79/actionview/lib/action_view/railtie.rb#L66-L69 We shouldn't wait for AB::Base to be loaded in order to set these configuration. However, we need to do it inside an `after_initialize` block in order to let application set it to the value they want. Closes #28538 Co-authored-by: betesh " --- .../metal/etag_with_template_digest.rb | 6 +- .../middleware/debug_exceptions.rb | 1 - actionview/lib/action_view.rb | 1 + actionview/lib/action_view/railtie.rb | 70 +++++++++---------- railties/test/application/loading_test.rb | 2 +- 5 files changed, 37 insertions(+), 43 deletions(-) diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb index 2f1544c69c..983bd745bd 100644 --- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb +++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb @@ -26,10 +26,8 @@ module ActionController included do class_attribute :etag_with_template_digest, default: true - ActiveSupport.on_load :action_view, yield: true do - etag do |options| - determine_template_etag(options) if etag_with_template_digest - end + etag do |options| + determine_template_etag(options) if etag_with_template_digest end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index e546d1c11f..4f3456738e 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -5,7 +5,6 @@ require "action_dispatch/middleware/exception_wrapper" require "action_dispatch/routing/inspector" require "action_view" -require "action_view/base" module ActionDispatch # This middleware is responsible for logging exceptions and diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index 11b4563548..cb638d1f4d 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -58,6 +58,7 @@ module ActionView autoload_at "action_view/template/resolver" do autoload :Resolver autoload :PathResolver + autoload :FileSystemResolver autoload :OptimizedFileSystemResolver autoload :FallbackFileSystemResolver end diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index 8bd526cdf9..5744b57a83 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -16,34 +16,34 @@ module ActionView config.eager_load_namespaces << ActionView - initializer "action_view.embed_authenticity_token_in_remote_forms" do |app| - ActiveSupport.on_load(:action_view) do - ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = - app.config.action_view.delete(:embed_authenticity_token_in_remote_forms) + config.after_initialize do |app| + ActionView::Helpers::FormTagHelper.embed_authenticity_token_in_remote_forms = + app.config.action_view.delete(:embed_authenticity_token_in_remote_forms) + end + + config.after_initialize do |app| + form_with_generates_remote_forms = app.config.action_view.delete(:form_with_generates_remote_forms) + ActionView::Helpers::FormHelper.form_with_generates_remote_forms = form_with_generates_remote_forms + end + + config.after_initialize do |app| + form_with_generates_ids = app.config.action_view.delete(:form_with_generates_ids) + unless form_with_generates_ids.nil? + ActionView::Helpers::FormHelper.form_with_generates_ids = form_with_generates_ids end end - initializer "action_view.form_with_generates_remote_forms" do |app| - ActiveSupport.on_load(:action_view) do - form_with_generates_remote_forms = app.config.action_view.delete(:form_with_generates_remote_forms) - ActionView::Helpers::FormHelper.form_with_generates_remote_forms = form_with_generates_remote_forms + config.after_initialize do |app| + default_enforce_utf8 = app.config.action_view.delete(:default_enforce_utf8) + unless default_enforce_utf8.nil? + ActionView::Helpers::FormTagHelper.default_enforce_utf8 = default_enforce_utf8 end end - initializer "action_view.form_with_generates_ids" do |app| + config.after_initialize do |app| ActiveSupport.on_load(:action_view) do - form_with_generates_ids = app.config.action_view.delete(:form_with_generates_ids) - unless form_with_generates_ids.nil? - ActionView::Helpers::FormHelper.form_with_generates_ids = form_with_generates_ids - end - end - end - - initializer "action_view.default_enforce_utf8" do |app| - ActiveSupport.on_load(:action_view) do - default_enforce_utf8 = app.config.action_view.delete(:default_enforce_utf8) - unless default_enforce_utf8.nil? - ActionView::Helpers::FormTagHelper.default_enforce_utf8 = default_enforce_utf8 + app.config.action_view.each do |k, v| + send "#{k}=", v end end end @@ -62,14 +62,6 @@ module ActionView ActiveSupport.on_load(:action_view) { self.logger ||= Rails.logger } end - initializer "action_view.set_configs" do |app| - ActiveSupport.on_load(:action_view) do - app.config.action_view.each do |k, v| - send "#{k}=", v - end - end - end - initializer "action_view.caching" do |app| ActiveSupport.on_load(:action_view) do if app.config.action_view.cache_template_loading.nil? @@ -78,14 +70,6 @@ module ActionView end end - initializer "action_view.per_request_digest_cache" do |app| - ActiveSupport.on_load(:action_view) do - unless ActionView::Resolver.caching? - app.executor.to_run ActionView::CacheExpiry::Executor.new(watcher: app.config.file_watcher) - end - end - end - initializer "action_view.setup_action_pack" do |app| ActiveSupport.on_load(:action_controller) do ActionView::RoutingUrlFor.include(ActionDispatch::Routing::UrlFor) @@ -96,6 +80,18 @@ module ActionView PartialRenderer.collection_cache = app.config.action_controller.cache_store end + config.after_initialize do |app| + enable_caching = if app.config.action_view.cache_template_loading.nil? + app.config.cache_classes + else + app.config.action_view.cache_template_loading + end + + unless enable_caching + app.executor.to_run ActionView::CacheExpiry::Executor.new(watcher: app.config.file_watcher) + end + end + rake_tasks do |app| unless app.config.api_only load "action_view/tasks/cache_digests.rake" diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index 9ab400acfc..3965b0b987 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -378,7 +378,7 @@ class LoadingTest < ActiveSupport::TestCase test "frameworks aren't loaded during initialization" do app_file "config/initializers/raise_when_frameworks_load.rb", <<-RUBY - %i(action_controller action_mailer active_job active_record).each do |framework| + %i(action_controller action_mailer active_job active_record action_view).each do |framework| ActiveSupport.on_load(framework) { raise "\#{framework} loaded!" } end RUBY