Merge pull request #34494 from gmcgibbon/warn_root_conflict

Stop using unnamed roots on conflict

3 files changed, 22 insertions, 7 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 90cf989100..740c6db06f 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Raise an error on root route naming conflicts.
+
+    Raises an ArgumentError when multiple root routes are defined in the
+    same context instead of assigning nil names to subsequent roots.
+
+    *Gannon McGibbon*
+
 *   Allow rescue from parameter parse errors:
 
     ```
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 06ce165f76..421e2023c2 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -656,7 +656,7 @@ module ActionDispatch
 
         # Query if the following named route was already defined.
         def has_named_route?(name)
-          @set.named_routes.key? name
+          @set.named_routes.key?(name)
         end
 
         private
@@ -1952,9 +1952,7 @@ module ActionDispatch
           end
 
           def match_root_route(options)
-            name = has_named_route?(name_for_action(:root, nil)) ? nil : :root
-            args = ["/", { as: name, via: :get }.merge!(options)]
-
+            args = ["/", { as: :root, via: :get }.merge(options)]
             match(*args)
           end
       end
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index affc2d8497..4dffbd0db1 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3698,15 +3698,25 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
-  def test_multiple_roots
+  def test_multiple_roots_raises_error
+    ex = assert_raises(ArgumentError) {
+      draw do
+        root "pages#index", constraints: { host: "www.example.com" }
+        root "admin/pages#index", constraints: { host: "admin.example.com" }
+      end
+    }
+    assert_match(/Invalid route name, already in use: 'root'/, ex.message)
+  end
+
+  def test_multiple_named_roots
     draw do
       namespace :foo do
         root "pages#index", constraints: { host: "www.example.com" }
-        root "admin/pages#index", constraints: { host: "admin.example.com" }
+        root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root
       end
 
       root "pages#index", constraints: { host: "www.example.com" }
-      root "admin/pages#index", constraints: { host: "admin.example.com" }
+      root "admin/pages#index", constraints: { host: "admin.example.com" },  as: :admin_root
     end
 
     get "http://www.example.com/foo"