From 870377915af301c98a54f7f588e077610b2190aa Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Thu, 14 Feb 2019 15:12:57 -0800 Subject: Replace autoloader accessors with Rails.autoloaders.{main,once} Rails.autoloader and Rails.once_autoloader was just tentative API good enough for a first patch. Rails.autoloader is singular and does not convey in its name that there is another autoloader. That might be confusing, for example if you set a logger and miss traces. On the other hand, the name `once_autoloader` is very close to being horrible. Rails.autoloaders.main and Rails.autoloaders.once read better for my taste, and have a nice symmetry. Also, both "main" and "once" are four letters long, short and same length. They are tagged as "rails.main" and "rails.once", respectively. References #35235. --- Gemfile.lock | 4 +-- activesupport/activesupport.gemspec | 2 +- .../dependencies/zeitwerk_integration.rb | 10 +++----- railties/lib/rails.rb | 19 ++------------ railties/lib/rails/autoloaders.rb | 30 ++++++++++++++++++++++ railties/lib/rails/engine.rb | 2 +- railties/test/application/configuration_test.rb | 27 +++++++++++-------- .../test/application/zeitwerk_integration_test.rb | 28 ++++++++++---------- 8 files changed, 72 insertions(+), 50 deletions(-) create mode 100644 railties/lib/rails/autoloaders.rb diff --git a/Gemfile.lock b/Gemfile.lock index b89c64d34e..5901314183 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -70,7 +70,7 @@ PATH i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) - zeitwerk (~> 1.0) + zeitwerk (~> 1.1) rails (6.0.0.beta1) actioncable (= 6.0.0.beta1) actionmailbox (= 6.0.0.beta1) @@ -517,7 +517,7 @@ GEM websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (1.0.0) + zeitwerk (1.1.0) PLATFORMS java diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index 92cdfd89fe..0879660f9e 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -34,5 +34,5 @@ Gem::Specification.new do |s| s.add_dependency "tzinfo", "~> 1.1" s.add_dependency "minitest", "~> 5.1" s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2" - s.add_dependency "zeitwerk", "~> 1.0" + s.add_dependency "zeitwerk", "~> 1.1" end diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index 75624bae09..55fc92ee8b 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -6,7 +6,7 @@ module ActiveSupport module Decorations def clear Dependencies.unload_interlock do - Rails.autoloader.reload + Rails.autoloaders.main.reload end end @@ -19,9 +19,7 @@ module ActiveSupport end def autoloaded_constants - Rails.autoloaders.flat_map do |autoloader| - autoloader.loaded.to_a - end + (Rails.autoloaders.main.loaded + Rails.autoloaders.once.loaded).to_a end def autoloaded?(object) @@ -46,9 +44,9 @@ module ActiveSupport next unless File.directory?(autoload_path) if autoload_once?(autoload_path) - Rails.once_autoloader.push_dir(autoload_path) + Rails.autoloaders.once.push_dir(autoload_path) else - Rails.autoloader.push_dir(autoload_path) + Rails.autoloaders.main.push_dir(autoload_path) end end diff --git a/railties/lib/rails.rb b/railties/lib/rails.rb index bca2cf34e1..1f533a8c04 100644 --- a/railties/lib/rails.rb +++ b/railties/lib/rails.rb @@ -13,6 +13,7 @@ require "active_support/core_ext/object/blank" require "rails/application" require "rails/version" +require "rails/autoloaders" require "active_support/railtie" require "action_dispatch/railtie" @@ -111,24 +112,8 @@ module Rails application && Pathname.new(application.paths["public"].first) end - def autoloader - if configuration.autoloader == :zeitwerk - @autoloader ||= Zeitwerk::Loader.new - end - end - - def once_autoloader - if configuration.autoloader == :zeitwerk - @once_autoloader ||= Zeitwerk::Loader.new - end - end - def autoloaders - if configuration.autoloader == :zeitwerk - [autoloader, once_autoloader] - else - [] - end + Autoloaders end end end diff --git a/railties/lib/rails/autoloaders.rb b/railties/lib/rails/autoloaders.rb new file mode 100644 index 0000000000..4b71f90c6b --- /dev/null +++ b/railties/lib/rails/autoloaders.rb @@ -0,0 +1,30 @@ +module Rails + module Autoloaders # :nodoc: + class << self + include Enumerable + + def main + if zeitwerk_enabled? + @main ||= Zeitwerk::Loader.new.tap { |loader| loader.tag = "rails.main" } + end + end + + def once + if zeitwerk_enabled? + @once ||= Zeitwerk::Loader.new.tap { |loader| loader.tag = "rails.once" } + end + end + + def each + if zeitwerk_enabled? + yield main + yield once + end + end + + def zeitwerk_enabled? + Rails.configuration.autoloader == :zeitwerk + end + end + end +end diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 2485158a7b..76be6a8ac7 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -472,7 +472,7 @@ module Rails # Eager load the application by loading all ruby # files inside eager_load paths. def eager_load! - if Rails.autoloader + if Rails.autoloaders.zeitwerk_enabled? eager_load_with_zeitwerk! else eager_load_with_dependencies! diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index f3161ad2a8..73773602a3 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1157,23 +1157,30 @@ module ApplicationTests end end - test "autoloader & autoloader=" do + test "autoloaders" do app "development" config = Rails.application.config - assert_instance_of Zeitwerk::Loader, Rails.autoloader - assert_instance_of Zeitwerk::Loader, Rails.once_autoloader - assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + assert Rails.autoloaders.zeitwerk_enabled? + assert_instance_of Zeitwerk::Loader, Rails.autoloaders.main + assert_equal "rails.main", Rails.autoloaders.main.tag + assert_instance_of Zeitwerk::Loader, Rails.autoloaders.once + assert_equal "rails.once", Rails.autoloaders.once.tag + assert_equal [Rails.autoloaders.main, Rails.autoloaders.once], Rails.autoloaders.to_a config.autoloader = :classic - assert_nil Rails.autoloader - assert_nil Rails.once_autoloader - assert_empty Rails.autoloaders + assert_not Rails.autoloaders.zeitwerk_enabled? + assert_nil Rails.autoloaders.main + assert_nil Rails.autoloaders.once + assert_equal 0, Rails.autoloaders.count config.autoloader = :zeitwerk - assert_instance_of Zeitwerk::Loader, Rails.autoloader - assert_instance_of Zeitwerk::Loader, Rails.once_autoloader - assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + assert Rails.autoloaders.zeitwerk_enabled? + assert_instance_of Zeitwerk::Loader, Rails.autoloaders.main + assert_equal "rails.main", Rails.autoloaders.main.tag + assert_instance_of Zeitwerk::Loader, Rails.autoloaders.once + assert_equal "rails.once", Rails.autoloaders.once.tag + assert_equal [Rails.autoloaders.main, Rails.autoloaders.once], Rails.autoloaders.to_a assert_raises(ArgumentError) { config.autoloader = :unknown } end diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index 628a85acd8..16ffbe56bb 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -30,9 +30,10 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase boot assert decorated? - assert_instance_of Zeitwerk::Loader, Rails.autoloader - assert_instance_of Zeitwerk::Loader, Rails.once_autoloader - assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + assert Rails.autoloaders.zeitwerk_enabled? + assert_instance_of Zeitwerk::Loader, Rails.autoloaders.main + assert_instance_of Zeitwerk::Loader, Rails.autoloaders.once + assert_equal [Rails.autoloaders.main, Rails.autoloaders.once], Rails.autoloaders.to_a end test "ActiveSupport::Dependencies is not decorated in classic mode" do @@ -40,9 +41,10 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase boot assert_not decorated? - assert_nil Rails.autoloader - assert_nil Rails.once_autoloader - assert_empty Rails.autoloaders + assert_not Rails.autoloaders.zeitwerk_enabled? + assert_nil Rails.autoloaders.main + assert_nil Rails.autoloaders.once + assert_equal 0, Rails.autoloaders.count end test "constantize returns the value stored in the constant" do @@ -136,8 +138,8 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase ) boot - assert_not_includes Rails.autoloader.dirs, "#{app_path}/extras" - assert_includes Rails.once_autoloader.dirs, "#{app_path}/extras" + assert_not_includes Rails.autoloaders.main.dirs, "#{app_path}/extras" + assert_includes Rails.autoloaders.once.dirs, "#{app_path}/extras" end test "clear reloads the main autoloader, and does not reload the once one" do @@ -145,13 +147,13 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase $zeitwerk_integration_reload_test = [] - autoloader = Rails.autoloader - def autoloader.reload - $zeitwerk_integration_reload_test << :autoloader + main_autoloader = Rails.autoloaders.main + def main_autoloader.reload + $zeitwerk_integration_reload_test << :main_autoloader super end - once_autoloader = Rails.once_autoloader + once_autoloader = Rails.autoloaders.once def once_autoloader.reload $zeitwerk_integration_reload_test << :once_autoloader super @@ -159,6 +161,6 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase ActiveSupport::Dependencies.clear - assert_equal %i(autoloader), $zeitwerk_integration_reload_test + assert_equal %i(main_autoloader), $zeitwerk_integration_reload_test end end -- cgit v1.2.3