Skip to content

Commit

Permalink
Avoid loading ActionView::Base during initialization (#1528)
Browse files Browse the repository at this point in the history
* Force tests to fail if Rails frameworks load during initialization

Couldn't see a clean way of running this inside of an actual TestCase
test, so took the nuclear option & fail the entire suite.

* Avoid loading ViewComponent::Base during initialization

It forces ActionView::Base to load, which is against Rails' engine
guidelines https://edgeguides.rubyonrails.org/engines.html#load-and-configuration-hooks

* Update docs/CHANGELOG.md

Co-authored-by: Joel Hawksley <joelhawksley@github.com>
Co-authored-by: Joel Hawksley <joel@hawksley.org>
  • Loading branch information
3 people authored Oct 11, 2022
1 parent 7d1b344 commit 22dbe8f
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 6 deletions.
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ nav_order: 5

## main

* Avoid loading ActionView::Base during Rails initialization.

*Jonathan del Strother*

## 2.74.1

* Add more users of ViewComponent to docs.
Expand Down
3 changes: 2 additions & 1 deletion lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ class << self
delegate(*ViewComponent::Config.defaults.keys, to: :config)

def config
@config ||= ViewComponent::Config.defaults
@config ||= ActiveSupport::OrderedOptions.new
end
attr_writer :config
end

include ViewComponent::ContentAreas
Expand Down
9 changes: 4 additions & 5 deletions lib/view_component/engine.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# frozen_string_literal: true

require "rails"
require "view_component/base"
require "view_component/config"

module ViewComponent
class Engine < Rails::Engine # :nodoc:
config.view_component = ViewComponent::Base.config
config.view_component = ViewComponent::Config.defaults

rake_tasks do
load "view_component/rails/tasks/view_component.rake"
Expand All @@ -14,9 +14,6 @@ class Engine < Rails::Engine # :nodoc:
initializer "view_component.set_configs" do |app|
options = app.config.view_component

%i[generate preview_controller preview_route show_previews_source].each do |config_option|
options[config_option] ||= ViewComponent::Base.public_send(config_option)
end
options.instrumentation_enabled = false if options.instrumentation_enabled.nil?
options.render_monkey_patch_enabled = true if options.render_monkey_patch_enabled.nil?
options.show_previews = (Rails.env.development? || Rails.env.test?) if options.show_previews.nil?
Expand All @@ -39,6 +36,8 @@ class Engine < Rails::Engine # :nodoc:

initializer "view_component.enable_instrumentation" do |app|
ActiveSupport.on_load(:view_component) do
Base.config = app.config.view_component

if app.config.view_component.instrumentation_enabled.present?
# :nocov:
ViewComponent::Base.prepend(ViewComponent::Instrumentation)
Expand Down
18 changes: 18 additions & 0 deletions test/sandbox/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,24 @@
require "action_view/railtie"
require "sprockets/railtie"

# Track when different Rails frameworks get loaded.
# Ideally, none of them should be loaded until after initialization is complete.
# See config/initializers/zzz_complete_initialization.rb for the other half of this.
RAILS_FRAMEWORKS = [
:action_cable,
:action_controller,
:action_mailer,
:action_view,
:active_job,
:active_record
]
FRAMEWORK_LOAD_POINTS = {}
RAILS_FRAMEWORKS.each do |feature|
ActiveSupport.on_load(feature) do
FRAMEWORK_LOAD_POINTS[feature] = caller
end
end

require "view_component"

require "haml"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

# FRAMEWORK_LOAD_POINTS ought to be empty - we shouldn't have
# autoloaded ActionView::Base during initialization, for example.
FRAMEWORK_LOAD_POINTS.each do |framework, caller|
warn "#{framework} loaded too soon, from:"
warn caller
exit 1
end

0 comments on commit 22dbe8f

Please sign in to comment.