Merge pull request #34064 from schneems/schneems/revert-7f870a5ba2aa9177aa4a0e03a9d027928ba60e49

Revert "Merge pull request #33970 from rails/eager-url-helpers"

8 files changed, 32 insertions, 67 deletions

diff --git a/actionpack/lib/abstract_controller/railties/routes_helpers.rb b/actionpack/lib/abstract_controller/railties/routes_helpers.rb
index c97be074c8..b6e5631a4e 100644
--- a/actionpack/lib/abstract_controller/railties/routes_helpers.rb
+++ b/actionpack/lib/abstract_controller/railties/routes_helpers.rb
@@ -7,8 +7,11 @@ module AbstractController
         Module.new do
           define_method(:inherited) do |klass|
             super(klass)
-
-            routes.include_helpers klass, include_path_helpers
+            if namespace = klass.parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) }
+              klass.include(namespace.railtie_routes_url_helpers(include_path_helpers))
+            else
+              klass.include(routes.url_helpers(include_path_helpers))
+            end
           end
         end
       end
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 2f68cefa94..99f3b4c2cd 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -676,6 +676,7 @@ module ActionDispatch
           def define_generate_prefix(app, name)
             _route = @set.named_routes.get name
             _routes = @set
+            _url_helpers = @set.url_helpers
 
             script_namer = ->(options) do
               prefix_options = options.slice(*_route.segment_keys)
@@ -687,7 +688,7 @@ module ActionDispatch
 
               # We must actually delete prefix segment keys to avoid passing them to next url_for.
               _route.segment_keys.each { |k| options.delete(k) }
-              @set.url_helpers.send("#{name}_path", prefix_options)
+              _url_helpers.send("#{name}_path", prefix_options)
             end
 
             app.routes.define_mounted_helper(name, script_namer)
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 2c811b5e55..fedf622e90 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -458,11 +458,6 @@ module ActionDispatch
         return if @finalized
         @append.each { |blk| eval_block(blk) }
         @finalized = true
-        @url_helpers = build_url_helper_module true
-        @deferred_classes.each { |klass, include_path_helpers|
-          include_helpers klass, include_path_helpers
-        }
-        @deferred_classes.clear
       end
 
       def clear!
@@ -491,10 +486,11 @@ module ActionDispatch
         return if MountedHelpers.method_defined?(name)
 
         routes = self
+        helpers = routes.url_helpers
 
         MountedHelpers.class_eval do
           define_method "_#{name}" do
-            RoutesProxy.new(routes, _routes_context, routes.url_helpers, script_namer)
+            RoutesProxy.new(routes, _routes_context, helpers, script_namer)
           end
         end
 
@@ -505,20 +501,7 @@ module ActionDispatch
         RUBY
       end
 
-      class UnfinalizedRouteSet < StandardError
-      end
-
       def url_helpers(supports_path = true)
-        raise UnfinalizedRouteSet, "routes have not been finalized. Please call `finalize!` or use `draw(&block)`" unless @finalized
-
-        if supports_path
-          @url_helpers
-        else
-          build_url_helper_module false
-        end
-      end
-
-      def build_url_helper_module(supports_path)
         routes = self
 
         Module.new do
diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
index 0e8712f8d9..af41521c5c 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
@@ -138,20 +138,6 @@ module ActionDispatch
         assert_generates(path.is_a?(Hash) ? path[:path] : path, generate_options, defaults, extras, message)
       end
 
-      # Provides a hook on `finalize!` so we can mutate a controller after the
-      # route set has been drawn.
-      class WithRouting < ActionDispatch::Routing::RouteSet # :nodoc:
-        def initialize(&block)
-          super()
-          @block = block
-        end
-
-        def finalize!
-          super
-          @block.call self
-        end
-      end
-
       # A helper to make it easier to test different route configurations.
       # This method temporarily replaces @routes with a new RouteSet instance.
       #
@@ -166,19 +152,16 @@ module ActionDispatch
       #   end
       #
       def with_routing
-        old_routes = @routes
-        old_controller = nil
-        @routes = WithRouting.new do |_routes|
-          if defined?(@controller) && @controller
-            old_controller, @controller = @controller, @controller.clone
-            _routes = @routes
-
-            @controller.singleton_class.include(_routes.url_helpers)
-
-            if @controller.respond_to? :view_context_class
-              @controller.view_context_class = Class.new(@controller.view_context_class) do
-                include _routes.url_helpers
-              end
+        old_routes, @routes = @routes, ActionDispatch::Routing::RouteSet.new
+        if defined?(@controller) && @controller
+          old_controller, @controller = @controller, @controller.clone
+          _routes = @routes
+
+          @controller.singleton_class.include(_routes.url_helpers)
+
+          if @controller.respond_to? :view_context_class
+            @controller.view_context_class = Class.new(@controller.view_context_class) do
+              include _routes.url_helpers
             end
           end
         end
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 7c9f15108d..65dd28b3d7 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -100,10 +100,7 @@ end
 
 class ActionDispatch::IntegrationTest < ActiveSupport::TestCase
   def self.build_app(routes = nil)
-    routes ||= ActionDispatch::Routing::RouteSet.new.tap { |rs|
-      rs.draw { }
-    }
-    RoutedRackApp.new(routes) do |middleware|
+    RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware|
       middleware.use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public")
       middleware.use ActionDispatch::DebugExceptions
       middleware.use ActionDispatch::Callbacks
diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb
index b078e8ad9f..39ede1442a 100644
--- a/actionpack/test/controller/integration_test.rb
+++ b/actionpack/test/controller/integration_test.rb
@@ -542,6 +542,9 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest
     def with_test_route_set
       with_routing do |set|
         controller = ::IntegrationProcessTest::IntegrationController.clone
+        controller.class_eval do
+          include set.url_helpers
+        end
 
         set.draw do
           get "moved" => redirect("/method")
@@ -552,10 +555,6 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest
           end
         end
 
-        controller.class_eval do
-          include set.url_helpers
-        end
-
         singleton_class.include(set.url_helpers)
 
         yield
diff --git a/actionpack/test/dispatch/routing/ipv6_redirect_test.rb b/actionpack/test/dispatch/routing/ipv6_redirect_test.rb
index fb423d2951..31559bffc7 100644
--- a/actionpack/test/dispatch/routing/ipv6_redirect_test.rb
+++ b/actionpack/test/dispatch/routing/ipv6_redirect_test.rb
@@ -4,11 +4,6 @@ require "abstract_unit"
 
 class IPv6IntegrationTest < ActionDispatch::IntegrationTest
   Routes = ActionDispatch::Routing::RouteSet.new
-  Routes.draw do
-    get "/",    to: "bad_route_request#index", as: :index
-    get "/foo", to: "bad_route_request#foo", as: :foo
-  end
-
   include Routes.url_helpers
 
   class ::BadRouteRequestController < ActionController::Base
@@ -22,6 +17,11 @@ class IPv6IntegrationTest < ActionDispatch::IntegrationTest
     end
   end
 
+  Routes.draw do
+    get "/",    to: "bad_route_request#index", as: :index
+    get "/foo", to: "bad_route_request#foo", as: :foo
+  end
+
   def _routes
     Routes
   end
diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb
index b0096d26be..aef9351de1 100644
--- a/actionpack/test/dispatch/url_generation_test.rb
+++ b/actionpack/test/dispatch/url_generation_test.rb
@@ -4,13 +4,16 @@ require "abstract_unit"
 
 module TestUrlGeneration
   class WithMountPoint < ActionDispatch::IntegrationTest
+    Routes = ActionDispatch::Routing::RouteSet.new
+    include Routes.url_helpers
+
     class ::MyRouteGeneratingController < ActionController::Base
+      include Routes.url_helpers
       def index
         render plain: foo_path
       end
     end
 
-    Routes = ActionDispatch::Routing::RouteSet.new
     Routes.draw do
       get "/foo", to: "my_route_generating#index", as: :foo
 
@@ -19,10 +22,6 @@ module TestUrlGeneration
       mount MyRouteGeneratingController.action(:index), at: "/bar"
     end
 
-    class ::MyRouteGeneratingController
-      include Routes.url_helpers
-    end
-
     APP = build_app Routes
 
     def _routes