Skip to content

Commit

Permalink
Do not memoize auto/eager load paths in engines
Browse files Browse the repository at this point in the history
  • Loading branch information
fxn committed Oct 14, 2023
1 parent 0ed3c2a commit ebfca90
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 11 deletions.
10 changes: 5 additions & 5 deletions railties/lib/rails/engine.rb
Expand Up @@ -578,7 +578,7 @@ def load_seed
end

initializer :set_eager_load_paths, before: :bootstrap_hook do
ActiveSupport::Dependencies._eager_load_paths.merge(config.eager_load_paths)
ActiveSupport::Dependencies._eager_load_paths.merge(config.all_eager_load_paths)
config.eager_load_paths.freeze
end

Expand Down Expand Up @@ -705,14 +705,14 @@ def default_middleware_stack
end

def _all_autoload_once_paths
config.autoload_once_paths.uniq
config.all_autoload_once_paths.uniq
end

def _all_autoload_paths
@_all_autoload_paths ||= begin
autoload_paths = config.autoload_paths
autoload_paths += config.eager_load_paths
autoload_paths -= config.autoload_once_paths
autoload_paths = config.all_autoload_paths
autoload_paths += config.all_eager_load_paths
autoload_paths -= config.all_autoload_once_paths
autoload_paths.uniq
end
end
Expand Down
51 changes: 45 additions & 6 deletions railties/lib/rails/engine/configuration.rb
Expand Up @@ -9,12 +9,45 @@ class Configuration < ::Rails::Railtie::Configuration
attr_accessor :middleware, :javascript_path
attr_writer :eager_load_paths, :autoload_once_paths, :autoload_paths

# An array of custom autoload paths to be added to the ones defined
# automatically by Rails. These won't be eager loaded, unless you push
# them to +eager_load_paths+ too, which is recommended.
#
# This collection is empty by default, it accepts strings and +Pathname+
# objects.
#
# If you'd like to add +lib+ to it, please see +autoload_lib+.
attr_reader :autoload_paths

# An array of custom autoload once paths. These won't be eager loaded
# unless you push them to +eager_load_paths+ too, which is recommended.
#
# This collection is empty by default, it accepts strings and +Pathname+
# objects.
#
# If you'd like to add +lib+ to it, please see +autoload_lib_once+.
attr_reader :autoload_once_paths

# An array of custom eager load paths to be added to the ones defined
# automatically by Rails. Anything in this collection is considered to be
# an autoload path regardless of whether it was added to +autoload_paths+.
#
# This collection is empty by default, it accepts strings and +Pathname+
# objects.
#
# If you'd like to add +lib+ to it, please see +autoload_lib+.
attr_reader :eager_load_paths

def initialize(root = nil)
super()
@root = root
@generators = app_generators.dup
@middleware = Rails::Configuration::MiddlewareStackProxy.new
@javascript_path = "javascript"

@autoload_paths = []
@autoload_once_paths = []
@eager_load_paths = []
end

# Holds generators configuration:
Expand Down Expand Up @@ -81,16 +114,22 @@ def root=(value)
@root = paths.path = Pathname.new(value).expand_path
end

def eager_load_paths
@eager_load_paths ||= paths.eager_load
# Private method that adds custom autoload paths to the ones defined by
# +paths+.
def all_autoload_paths # :nodoc:
autoload_paths + paths.autoload_paths
end

def autoload_once_paths
@autoload_once_paths ||= paths.autoload_once
# Private method that adds custom autoload once paths to the ones defined
# by +paths+.
def all_autoload_once_paths # :nodoc:
autoload_once_paths + paths.autoload_once
end

def autoload_paths
@autoload_paths ||= paths.autoload_paths
# Private method that adds custom eager load paths to the ones defined by
# +paths+.
def all_eager_load_paths # :nodoc:
eager_load_paths + paths.eager_load
end
end
end
Expand Down
24 changes: 24 additions & 0 deletions railties/test/application/configuration_test.rb
Expand Up @@ -662,6 +662,30 @@ def assert_utf8
assert_utf8
end

# Regression test for https://github.com/rails/rails/issues/49629.
test "config.paths can be mutated after accessing auto/eager load paths" do
app_dir "vendor/auto"
app_dir "vendor/once"
app_dir "vendor/eager"

add_to_config <<~RUBY
# Reading the collections is enough, no need to modify them.
config.autoload_paths
config.autoload_once_paths
config.eager_load_paths
config.paths.add("vendor/auto", autoload: true)
config.paths.add("vendor/once", autoload_once: true)
config.paths.add("vendor/eager", eager_load: true)
RUBY

app "development"

assert_includes ActiveSupport::Dependencies.autoload_paths, "#{Rails.root}/vendor/auto"
assert_includes ActiveSupport::Dependencies.autoload_once_paths, "#{Rails.root}/vendor/once"
assert_includes ActiveSupport::Dependencies._eager_load_paths, "#{Rails.root}/vendor/eager"
end

test "config.paths.public sets Rails.public_path" do
add_to_config <<-RUBY
config.paths["public"] = "somewhere"
Expand Down

0 comments on commit ebfca90

Please sign in to comment.