From 278f0dfeac03e9e9d57eae8f4618ff94d8584abe Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Sat, 30 Mar 2019 08:38:41 +0100 Subject: fixes eager loading edge case in :zeitwerk mode --- activesupport/CHANGELOG.md | 9 +++++++ activesupport/lib/active_support/dependencies.rb | 5 ++++ .../dependencies/zeitwerk_integration.rb | 20 ++++++++++------ railties/lib/rails/engine.rb | 7 ++++-- .../test/application/zeitwerk_integration_test.rb | 28 ++++++++++++++++++++++ 5 files changed, 60 insertions(+), 9 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index f30249cb3f..de933ec122 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,12 @@ +* In `:zeitwerk` mode, eager load directories in engines and applications only + if present in their respective `config.eager_load_paths`. + + A common use case for this is adding `lib` to `config.autoload_paths`, but + not to `config.eager_load_paths`. In that configuration, for example, files + in the `lib` directory should not be eager loaded. + + *Xavier Noria* + * Fix bug in Range comparisons when comparing to an excluded-end Range Before: diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index d5d00b5e6e..82f07c085e 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -70,6 +70,11 @@ module ActiveSupport #:nodoc: # only once. All directories in this set must also be present in +autoload_paths+. mattr_accessor :autoload_once_paths, default: [] + # This is a private set that collects all eager load paths during bootstrap. + # Useful for Zeitwerk integration. Its public interface is the config.* path + # accessors of each engine. + mattr_accessor :_eager_load_paths, default: Set.new + # An array of qualified constant names that have been loaded. Adding a name # to this array will cause it to be unloaded the next time Dependencies are # cleared. diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index c6fdade006..a43d03cf09 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "set" require "active_support/core_ext/string/inflections" module ActiveSupport @@ -52,7 +53,7 @@ module ActiveSupport class << self def take_over setup_autoloaders - freeze_autoload_paths + freeze_paths decorate_dependencies end @@ -64,11 +65,11 @@ module ActiveSupport # prevent misconfigurations. next unless File.directory?(autoload_path) - if autoload_once?(autoload_path) - Rails.autoloaders.once.push_dir(autoload_path) - else - Rails.autoloaders.main.push_dir(autoload_path) - end + autoloader = \ + autoload_once?(autoload_path) ? Rails.autoloaders.once : Rails.autoloaders.main + + autoloader.push_dir(autoload_path) + autoloader.do_not_eager_load(autoload_path) unless eager_load?(autoload_path) end Rails.autoloaders.each(&:setup) @@ -78,9 +79,14 @@ module ActiveSupport Dependencies.autoload_once_paths.include?(autoload_path) end - def freeze_autoload_paths + def eager_load?(autoload_path) + Dependencies._eager_load_paths.member?(autoload_path) + end + + def freeze_paths Dependencies.autoload_paths.freeze Dependencies.autoload_once_paths.freeze + Dependencies._eager_load_paths.freeze end def decorate_dependencies diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 07bd56c978..2b5afd51b8 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -567,12 +567,15 @@ module Rails ActiveSupport::Dependencies.autoload_paths.unshift(*_all_autoload_paths) ActiveSupport::Dependencies.autoload_once_paths.unshift(*_all_autoload_once_paths) - # Freeze so future modifications will fail rather than do nothing mysteriously config.autoload_paths.freeze - config.eager_load_paths.freeze config.autoload_once_paths.freeze end + initializer :set_eager_load_paths, before: :bootstrap_hook do + ActiveSupport::Dependencies._eager_load_paths.merge(config.eager_load_paths) + config.eager_load_paths.freeze + end + initializer :add_routing_paths do |app| routing_paths = paths["config/routes.rb"].existent diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index c82b37d07d..e4a17e5a31 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -149,6 +149,34 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert $zeitwerk_integration_test_extras end + test "autoload directores not present in eager load paths are not eager loaded" do + $zeitwerk_integration_test_user = false + app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" + + $zeitwerk_integration_test_lib = false + app_dir "lib" + app_file "lib/webhook_hacks.rb", "WebhookHacks = 1; $zeitwerk_integration_test_lib = true" + + $zeitwerk_integration_test_extras = false + app_dir "extras" + app_file "extras/websocket_hacks.rb", "WebsocketHacks = 1; $zeitwerk_integration_test_extras = true" + + add_to_config "config.autoload_paths << '#{app_path}/lib'" + add_to_config "config.autoload_once_paths << '#{app_path}/extras'" + + boot("production") + + assert $zeitwerk_integration_test_user + assert !$zeitwerk_integration_test_lib + assert !$zeitwerk_integration_test_extras + + assert WebhookHacks + assert WebsocketHacks + + assert $zeitwerk_integration_test_lib + assert $zeitwerk_integration_test_extras + end + test "autoload_paths are set as root dirs of main, and in the same order" do boot -- cgit v1.2.3