Permalink
Browse files

options already have symbolized keys, so we can avoid this call

  • Loading branch information...
1 parent 977d36a commit 566f25beeee1837099451d4bc314301d72c61503 @tenderlove tenderlove committed May 13, 2014
Showing with 1 addition and 1 deletion.
  1. +1 −1 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -228,7 +228,7 @@ def call(t, args)
options = handle_positional_args(t, args, @options, @segment_keys)
hash = {
:only_path => options[:host].nil?
- }.merge!(options.symbolize_keys)
+ }.merge!(options)
hash.reverse_merge!(t.url_options)
t._routes.url_for(hash)
end

3 comments on commit 566f25b

Owner

pixeltrix commented on 566f25b May 14, 2014

Are you sure this is true? After this commit this no longer works:

routes = ActionDispatch::Routing::RouteSet.new
routes.draw{ get '/posts', to: 'posts#index', as: 'posts' }
routes.url_helpers.posts_url('host' => 'www.example.com')

That's not to say it should work - there's an argument to be made about what the hell are people doing sending string keys into url helpers any way, but it needs to be a conscious choice that we make. I'd like to see more of this compatibility between string and symbol keys pushed back into the application layer so that everyone isn't paying the cost of it.

Owner

pixeltrix replied May 15, 2014

Okay, looks like it didn't work in 4.0 and 4.1 - you added the symbolize_keys in ffa53ff, sorry for the false alarm.

Member

sgrif replied Feb 2, 2015

Looks like this caused #18175

Please sign in to comment.