Merge pull request #43028 from rails/classic

Do not hook the classic autoloader anymore
This commit is contained in:
Xavier Noria 2021-08-17 06:51:00 +02:00 committed by GitHub
commit 1f566bd9ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 165 additions and 231 deletions

View File

@ -46,8 +46,6 @@ end
ActionPackTestSuiteUtils.require_helpers("#{__dir__}/fixtures/helpers")
ActionPackTestSuiteUtils.require_helpers("#{__dir__}/fixtures/alternate_helpers")
ActiveSupport::Dependencies.hook!
Thread.abort_on_exception = true
# Show backtraces for deprecated behavior for quicker cleanup.

View File

@ -87,8 +87,8 @@ end
class HelpersTypoControllerTest < ActiveSupport::TestCase
def test_helper_typo_error_message
e = assert_raise(NameError) { HelpersTypoController.helper "admin/users" }
# This message is better if autoloading.
assert_equal "uninitialized constant Admin::UsersHelper\nDid you mean? Admin::UsersHelpeR", e.message
assert_includes e.message, "uninitialized constant Admin::UsersHelper"
assert_includes e.message, "Did you mean? Admin::UsersHelpeR"
end
end

View File

@ -34,8 +34,6 @@ end
ActionViewTestSuiteUtils.require_helpers("#{__dir__}/fixtures/helpers")
ActionViewTestSuiteUtils.require_helpers("#{__dir__}/fixtures/alternate_helpers")
ActiveSupport::Dependencies.hook!
Thread.abort_on_exception = true
# Show backtraces for deprecated behavior for quicker cleanup.

View File

@ -71,10 +71,8 @@ module AbstractController
end
def test_declare_missing_helper
e = assert_raise NameError do
AbstractHelpers.helper :missing
end
assert_equal "uninitialized constant MissingHelper", e.message
e = assert_raise(NameError) { AbstractHelpers.helper :missing }
assert_includes e.message, "uninitialized constant MissingHelper"
end
def test_helpers_with_module_through_block
@ -103,7 +101,7 @@ module AbstractController
class InvalidHelpersTest < ActiveSupport::TestCase
def test_controller_raise_error_about_missing_helper
e = assert_raise(NameError) { AbstractHelpers.helper(:missing) }
assert_equal "uninitialized constant MissingHelper", e.message
assert_includes e.message, "uninitialized constant MissingHelper"
end
end
end

View File

@ -50,6 +50,10 @@ module ActiveSupport # :nodoc:
# :nodoc:
def eager_load?(path)
Dependencies._eager_load_paths.member?(path)
end
# All files ever loaded.
mattr_accessor :history, default: Set.new
@ -293,16 +297,6 @@ module ActiveSupport # :nodoc:
end
end
def hook!
Loadable.include_into(Object)
ModuleConstMissing.include_into(Module)
end
def unhook!
ModuleConstMissing.exclude_from(Module)
Loadable.exclude_from(Object)
end
def load?
mechanism == :load
end
@ -697,5 +691,3 @@ module ActiveSupport # :nodoc:
end
end
end
ActiveSupport::Dependencies.hook!

View File

@ -37,10 +37,6 @@ module ActiveSupport
l = verbose ? logger || Rails.logger : nil
Rails.autoloaders.each { |autoloader| autoloader.logger = l }
end
def unhook!
:no_op
end
end
module Inflector
@ -57,55 +53,8 @@ module ActiveSupport
end
end
class << self
def take_over(enable_reloading:)
setup_autoloaders(enable_reloading)
freeze_paths
decorate_dependencies
end
private
def setup_autoloaders(enable_reloading)
Dependencies.autoload_paths.each do |autoload_path|
# Zeitwerk only accepts existing directories in `push_dir` to
# prevent misconfigurations.
next unless File.directory?(autoload_path)
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
if enable_reloading
Rails.autoloaders.main.enable_reloading
Rails.autoloaders.main.on_unload do |_cpath, value, _abspath|
value.before_remove_const if value.respond_to?(:before_remove_const)
end
end
Rails.autoloaders.each(&:setup)
end
def autoload_once?(autoload_path)
Dependencies.autoload_once_paths.include?(autoload_path)
end
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
Dependencies.unhook!
Dependencies.singleton_class.prepend(Decorations)
end
def self.take_over
Dependencies.singleton_class.prepend(Decorations)
end
end
end

View File

@ -98,22 +98,6 @@ class DependenciesTest < ActiveSupport::TestCase
ActiveSupport::Dependencies.new_constants_in("Illegal-Name") { }
end
end
def test_hook_called_multiple_times
assert_nothing_raised { ActiveSupport::Dependencies.hook! }
end
def test_load_and_require_stay_private
assert_includes Object.private_methods, :load
assert_includes Object.private_methods, :require
ActiveSupport::Dependencies.unhook!
assert_includes Object.private_methods, :load
assert_includes Object.private_methods, :require
ensure
ActiveSupport::Dependencies.hook!
end
end
class RequireDependencyTest < ActiveSupport::TestCase

View File

@ -66,8 +66,8 @@ The section _Customizing Inflections_ below documents ways to override this defa
Please, check the [Zeitwerk documentation](https://github.com/fxn/zeitwerk#file-structure) for further details.
Autoload Paths
--------------
config.autoload_paths
---------------------
We refer to the list of application directories whose contents are to be autoloaded as _autoload paths_. For example, `app/models`. Such directories represent the root namespace: `Object`.
@ -86,10 +86,60 @@ UsersHelper
Autoload paths automatically pick up any custom directories under `app`. For example, if your application has `app/presenters`, or `app/services`, etc., they are added to autoload paths.
The array of autoload paths can be extended by mutating `config.autoload_paths`, in `config/application.rb`, but nowadays this is discouraged.
The array of autoload paths can be extended by pushing to `config.autoload_paths`, in `config/application.rb` or `config/environments/*.rb`.
WARNING. Please do not mutate `ActiveSupport::Dependencies.autoload_paths`; the public interface to change autoload paths is `config.autoload_paths`.
These paths are managed by the `Rails.autoloaders.main` autoloader.
WARNING: The classes and modules defined in the autoload paths are reloadable. Therefore, you cannot autoload them in initializers. Please check [_Autoloading when the application boots_](#autoloading-when-the-application-boots) down below for valid ways to do that.
config.autoload_once_paths
--------------------------
Occasionally, you may want to be able to autoload classes and modules without reloading them. This is key for classes and modules that are cached somewhere.
For example, Active Job serializers are stored inside Active Job:
```ruby
# config/initializer/custom_serializers.rb
Rails.application.config.active_job.custom_serializers << MoneySerializer
```
Making `MoneySerializer` reloadable would be confusing, because reloading an edited version would have no effect on that class object stored in Active Job. Indeed, if `MoneySerializer` was reloadable, starting with Rails 7 such initializer would raise a `NameError`.
Another use case are engines decorating framework classes:
```ruby
initializer "decorate ActionController::Base" do
ActiveSupport.on_load(:action_controller_base) do
include MyDecoration
end
end
```
There, the module object stored in `MyDecoration` by the time the initializer runs becomes an ancestor of `ActionController::Base`, and reloading `MyDecoration` is pointless, it won't affect that ancestor chain.
The directories in `config.autoload_once_paths` are managed by `Rails.autoloaders.once` and cover those use cases by allowing you to autoload classes and modules that won't be reloaded.
The array of autoload once paths can be extended by pushing to `config.autoload_once_paths`, in `config/application.rb` or `config/environments/*.rb`. For example:
```ruby
module MyApplication
class Application < Rails::Application
config.autoload_once_paths << "#{root}/app/serializers"
end
end
```
Classes and modules from the autoload once paths can be autoloaded in `config/initializers`. So, with that configuration this works:
```ruby
# config/initializer/custom_serializers.rb
Rails.application.config.active_job.custom_serializers << MoneySerializer
```
INFO: Technically, you can autoload classes and modules managed by the `once` autoloader in any initializer that runs after `:bootstrap_hook`.
$LOAD_PATH
----------

View File

@ -64,6 +64,23 @@ module Rails
end
end
# We setup the once autoloader this early so that engines and applications
# are able to autoload from these paths during initialization.
initializer :setup_once_autoloader do
autoloader = Rails.autoloaders.once
ActiveSupport::Dependencies.autoload_once_paths.freeze
ActiveSupport::Dependencies.autoload_once_paths.uniq.each do |path|
# Zeitwerk only accepts existing directories in `push_dir`.
next unless File.directory?(path)
autoloader.push_dir(path)
autoloader.do_not_eager_load(path) unless ActiveSupport::Dependencies.eager_load?(path)
end
autoloader.setup
end
initializer :bootstrap_hook, group: :all do |app|
ActiveSupport.run_load_hooks(:before_initialize, app)
end

View File

@ -13,68 +13,31 @@ module Rails
config.generators.templates.unshift(*paths["lib/templates"].existent)
end
initializer :ensure_autoload_once_paths_as_subset do
extra = ActiveSupport::Dependencies.autoload_once_paths -
ActiveSupport::Dependencies.autoload_paths
initializer :setup_main_autoloader do
autoloader = Rails.autoloaders.main
unless extra.empty?
abort <<-end_error
autoload_once_paths must be a subset of the autoload_paths.
Extra items in autoload_once_paths: #{extra * ','}
end_error
ActiveSupport::Dependencies.autoload_paths.freeze
ActiveSupport::Dependencies.autoload_paths.uniq.each do |path|
# Zeitwerk only accepts existing directories in `push_dir`.
next unless File.directory?(path)
autoloader.push_dir(path)
autoloader.do_not_eager_load(path) unless ActiveSupport::Dependencies.eager_load?(path)
end
end
# This will become an error if/when we remove classic mode. The plan is
# autoloaders won't be configured up to this point in the finisher, so
# constants just won't be found, raising regular NameError exceptions.
initializer :warn_if_autoloaded, before: :let_zeitwerk_take_over do
next if config.cache_classes
next if ActiveSupport::Dependencies.autoloaded_constants.empty?
unless config.cache_classes
autoloader.enable_reloading
autoloader.on_unload do |_cpath, value, _abspath|
value.before_remove_const if value.respond_to?(:before_remove_const)
end
end
autoloaded = ActiveSupport::Dependencies.autoloaded_constants
constants = "constant".pluralize(autoloaded.size)
enum = autoloaded.to_sentence
have = autoloaded.size == 1 ? "has" : "have"
these = autoloaded.size == 1 ? "This" : "These"
example = autoloaded.first
example_klass = example.constantize.class
ActiveSupport::DescendantsTracker.clear
ActiveSupport::Dependencies.clear
unload_message = "#{these} autoloaded #{constants} #{have} been unloaded."
ActiveSupport::Deprecation.warn(<<~WARNING)
Initialization autoloaded the #{constants} #{enum}.
Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.
Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload #{example}, for example,
the expected changes won't be reflected in that stale #{example_klass} object.
#{unload_message}
In order to autoload safely at boot time, please wrap your code in a reloader
callback this way:
Rails.application.reloader.to_prepare do
# Autoload classes and modules needed at boot time here.
end
That block runs when the application boots, and every time there is a reload.
For historical reasons, it may run twice, so it has to be idempotent.
Check the "Autoloading and Reloading Constants" guide to learn more about how
Rails autoloads and reloads.
WARNING
autoloader.setup
end
initializer :let_zeitwerk_take_over do
require "active_support/dependencies/zeitwerk_integration"
ActiveSupport::Dependencies::ZeitwerkIntegration.take_over(enable_reloading: !config.cache_classes)
ActiveSupport::Dependencies::ZeitwerkIntegration.take_over
end
# Setup default session store if not already set in config/application.rb
@ -245,13 +208,6 @@ module Rails
end
end
end
# Disable dependency loading during request cycle
initializer :disable_dependency_loading do
if config.eager_load && config.cache_classes && !config.enable_dependency_loading
ActiveSupport::Dependencies.unhook!
end
end
end
end
end

View File

@ -570,17 +570,14 @@ module Rails
$LOAD_PATH.uniq!
end
# Set the paths from which Rails will automatically load source files,
# and the load_once paths.
#
# This needs to be an initializer, since it needs to run once
# per engine and get the engine as a block parameter.
initializer :set_autoload_paths, before: :bootstrap_hook do
ActiveSupport::Dependencies.autoload_paths.unshift(*_all_autoload_paths)
ActiveSupport::Dependencies.autoload_once_paths.unshift(*_all_autoload_once_paths)
config.autoload_paths.freeze
initializer :set_autoload_once_paths, before: :setup_once_autoloader do
config.autoload_once_paths.freeze
ActiveSupport::Dependencies.autoload_once_paths.unshift(*_all_autoload_once_paths)
end
initializer :set_autoload_paths, before: :setup_main_autoloader do
config.autoload_paths.freeze
ActiveSupport::Dependencies.autoload_paths.unshift(*_all_autoload_paths)
end
initializer :set_eager_load_paths, before: :bootstrap_hook do
@ -694,17 +691,25 @@ module Rails
end
def _all_autoload_once_paths
config.autoload_once_paths
config.autoload_once_paths.uniq
end
def _all_autoload_paths
@_all_autoload_paths ||= (config.autoload_paths + config.eager_load_paths + config.autoload_once_paths).uniq
@_all_autoload_paths ||= begin
autoload_paths = config.autoload_paths
autoload_paths += config.eager_load_paths
autoload_paths -= config.autoload_once_paths
autoload_paths.uniq
end
end
def _all_load_paths(add_autoload_paths_to_load_path)
@_all_load_paths ||= begin
load_paths = config.paths.load_paths
load_paths += _all_autoload_paths if add_autoload_paths_to_load_path
load_paths = config.paths.load_paths
if add_autoload_paths_to_load_path
load_paths += _all_autoload_paths
load_paths += _all_autoload_once_paths
end
load_paths.uniq
end
end

View File

@ -1874,60 +1874,6 @@ module ApplicationTests
assert_includes $LOAD_PATH, "#{app_path}/custom_eager_load_path"
end
test "autoloading during initialization gets deprecation message and clearing if config.cache_classes is false" do
app_file "lib/c.rb", <<~EOS
class C
extend ActiveSupport::DescendantsTracker
end
class X < C
end
EOS
app_file "app/models/d.rb", <<~EOS
require "c"
class D < C
end
EOS
app_file "config/initializers/autoload.rb", "D.class"
app "development"
# TODO: Test deprecation message, assert_deprecated { app "development" }
# does not collect it.
assert_equal [X], C.descendants
assert_empty ActiveSupport::Dependencies.autoloaded_constants
end
test "autoloading during initialization triggers nothing if config.cache_classes is true" do
app_file "lib/c.rb", <<~EOS
class C
extend ActiveSupport::DescendantsTracker
end
class X < C
end
EOS
app_file "app/models/d.rb", <<~EOS
require "c"
class D < C
end
EOS
app_file "config/initializers/autoload.rb", "D.class"
app "production"
# TODO: Test no deprecation message is issued.
assert_equal [X, D], C.descendants
end
test "load_database_yaml returns blank hash if configuration file is blank" do
app_file "config/database.yml", ""
app "development"

View File

@ -96,6 +96,54 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase
assert_not deps.autoloaded?(invalid_constant_name)
end
test "the once autoloader can autoload from initializers" do
app_file "extras0/x.rb", "X = 0"
app_file "extras1/y.rb", "Y = 0"
# We should be able to configure autoload_once_paths in
# config/application.rb and in config/environments/*.rb.
add_to_config 'config.autoload_once_paths << "#{Rails.root}/extras0"'
add_to_env_config "development", 'config.autoload_once_paths << "#{Rails.root}/extras1"'
# Collections should br frozen after bootstrap, and you are ready to
# autoload with the once autoloader. In particular, from initializers.
$config_autoload_once_paths_is_frozen = false
$global_autoload_once_paths_is_frozen = false
add_to_config <<~RUBY
initializer :test_autoload_once_paths_is_frozen, after: :bootstrap_hook do
$config_autoload_once_paths_is_frozen = config.autoload_once_paths.frozen?
$global_autoload_once_paths_is_frozen = ActiveSupport::Dependencies.autoload_once_paths.frozen?
X
end
RUBY
app_file "config/initializers/autoload_Y.rb", "Y"
# Preconditions.
assert_not Object.const_defined?(:X)
assert_not Object.const_defined?(:Y)
boot
assert Object.const_defined?(:X)
assert Object.const_defined?(:Y)
assert $config_autoload_once_paths_is_frozen
assert $global_autoload_once_paths_is_frozen
end
test "the once autoloader can eager load" do
app_file "app/serializers/money_serializer.rb", "MoneySerializer = :dummy_value"
add_to_config 'config.autoload_once_paths << "#{Rails.root}/app/serializers"'
add_to_config 'config.eager_load_paths << "#{Rails.root}/app/serializers"'
assert_not Object.const_defined?(:MoneySerializer)
boot("production")
assert Object.const_defined?(:MoneySerializer)
end
test "unloadable constants (main)" do
app_file "app/models/user.rb", "class User; end"
app_file "app/models/post.rb", "class Post; end"
@ -305,13 +353,6 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase
assert_equal %i(main_autoloader), $zeitwerk_integration_reload_test
end
test "unhooks" do
boot
assert_equal Module, Module.method(:const_missing).owner
assert_equal :no_op, deps.unhook!
end
test "reloading invokes before_remove_const" do
$before_remove_const_invoked = false