clearing autoloaded constants triggers routes reloading [Fixes #10685]

Conflicts:
	railties/test/application/loading_test.rb

2 files changed, 61 insertions, 9 deletions

diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb
index 3ae60312c5..7a1bb1e25c 100644
--- a/railties/lib/rails/application/finisher.rb
+++ b/railties/lib/rails/application/finisher.rb
@@ -62,17 +62,28 @@ module Rails
         ActiveSupport.run_load_hooks(:after_initialize, self)
       end
 
-      # Set app reload just after the finisher hook to ensure
-      # routes added in the hook are still loaded.
+      # Set routes reload after the finisher hook to ensure routes added in
+      # the hook are taken into account.
       initializer :set_routes_reloader_hook do
         reloader = routes_reloader
         reloader.execute_if_updated
         self.reloaders << reloader
-        ActionDispatch::Reloader.to_prepare { reloader.execute_if_updated }
+        ActionDispatch::Reloader.to_prepare do
+          # We configure #execute rather than #execute_if_updated because if
+          # autoloaded constants are cleared we need to reload routes also in
+          # case any was used there, as in
+          #
+          #   mount MailPreview => 'mail_view'
+          #
+          # This means routes are also reloaded if i18n is updated, which
+          # might not be necessary, but in order to be more precise we need
+          # some sort of reloaders dependency support, to be added.
+          reloader.execute
+        end
       end
 
-      # Set app reload just after the finisher hook to ensure
-      # paths added in the hook are still loaded.
+      # Set clearing dependencies after the finisher hook to ensure paths
+      # added in the hook are taken into account.
       initializer :set_clear_dependencies_hook, group: :all do
         callback = lambda do
           ActiveSupport::DescendantsTracker.clear
@@ -82,9 +93,17 @@ module Rails
         if config.reload_classes_only_on_change
           reloader = config.file_watcher.new(*watchable_args, &callback)
           self.reloaders << reloader
-          # We need to set a to_prepare callback regardless of the reloader result, i.e.
-          # models should be reloaded if any of the reloaders (i18n, routes) were updated.
-          ActionDispatch::Reloader.to_prepare(prepend: true){ reloader.execute }
+
+          # Prepend this callback to have autoloaded constants cleared before
+          # any other possible reloading, in case they need to autoload fresh
+          # constants.
+          ActionDispatch::Reloader.to_prepare(prepend: true) do
+            # In addition to changes detected by the file watcher, if routes
+            # or i18n have been updated we also need to clear constants,
+            # that's why we run #execute rather than #execute_if_updated, this
+            # callback has to clear autoloaded constants after any update.
+            reloader.execute
+          end
         else
           ActionDispatch::Reloader.to_cleanup(&callback)
         end
diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb
index 96aa35b678..17926ac444 100644
--- a/railties/test/application/loading_test.rb
+++ b/railties/test/application/loading_test.rb
@@ -179,7 +179,7 @@ class LoadingTest < ActiveSupport::TestCase
     RUBY
 
     app_file 'config/routes.rb', <<-RUBY
-      $counter = 0
+      $counter ||= 0
       Rails.application.routes.draw do
         get '/c', to: lambda { |env| User; [200, {"Content-Type" => "text/plain"}, [$counter.to_s]] }
       end
@@ -205,6 +205,39 @@ class LoadingTest < ActiveSupport::TestCase
     assert_equal "2", last_response.body
   end
 
+  test "dependencies reloading is followed by routes reloading" do
+    add_to_config <<-RUBY
+      config.cache_classes = false
+    RUBY
+
+    app_file 'config/routes.rb', <<-RUBY
+      $counter ||= 1
+      $counter  *= 2
+      AppTemplate::Application.routes.draw do
+        get '/c', to: lambda { |env| User; [200, {"Content-Type" => "text/plain"}, [$counter.to_s]] }
+      end
+    RUBY
+
+    app_file "app/models/user.rb", <<-MODEL
+      class User
+        $counter += 1
+      end
+    MODEL
+
+    require 'rack/test'
+    extend Rack::Test::Methods
+
+    require "#{rails_root}/config/environment"
+
+    get "/c"
+    assert_equal "3", last_response.body
+
+    app_file "db/schema.rb", ""
+
+    get "/c"
+    assert_equal "7", last_response.body
+  end
+
   test "columns migrations also trigger reloading" do
     add_to_config <<-RUBY
       config.cache_classes = false