This project is archived and is in readonly mode.
Add optional :format argument to named routes
Reported by Tom Stuart | November 12th, 2008 @ 11:12 AM
The discussion in #1215 has investigated ways to reduce the number
of memory-hungry routes generated by
map.resources
.
Several people have suggested that we could abandon the
formatted_
named routes altogether and replace them
with a :format
parameter to the regular named routes.
This seems like a more natural implementation and will avoid
cluttering the route set for people who rarely or never use these
routes.
Related links oourtesy of aaronbatalion:
- http://blog.hungrymachine.com/20...
- http://gist.github.com/23712
- http://blog.hungrymachine.com/20...
Can we get a patch together?
Comments and changes to this ticket
-
DHH November 12th, 2008 @ 11:55 AM
I like this. Seems that http://gist.github.com/23712 has the pointers for making this so. Just needs to be wrapped up in a real patch with tests.
-
Michael Koziarski November 12th, 2008 @ 12:01 PM
At the same time it would be great to continue jeremy's work to cut down on the memory usage from the generated recognition code.
-
aaronbatalion November 12th, 2008 @ 12:08 PM
I was working on a real patch with tests already. Will update soon. There are a couple known bugs in the above gist.
-
aaronbatalion November 13th, 2008 @ 02:08 AM
- Tag changed from actionpack, options, resources to actionpack, options, patch, resources
Attached is the patch for optional .:format in routes, which decreases the number of routes by 50%, saving up to 100M of RAM on larger rails apps.
Notes: Found one side effect when PageCachingTest.
In RouteSet#routes_for_controller_and_action_and_keys, routes are sorted by significant keys, subtracting the keys that are passed in. Therefore, are UrlRewriter.rewrite(:controller => "foo", :format => nil) and UrlRewriter.rewrite(:controller => "foo") are not the same.
I've added a test to UrlRewriterTest, and modified the RouteSet#routes_for_controller_and_action_and_keys to remove pairs with nil values, and left the PageCachingTest alone.
-
Lourens Naudé November 13th, 2008 @ 05:42 PM
Aaron,
Attached is an updated diff, compatible with Tom's changes from http://github.com/rails/rails/co...
All tests passing and just piped it through the test suite of a large app with a huge number of nested routes.
All seems well.Great track of thought with this !
- Lourens
-
Michael Koziarski November 14th, 2008 @ 10:35 AM
Really nice work so far guys, if we do this we need to think about a few things:
generation optimisations
They assume that all segments are mandatory. This changes that and will cause them to fail to kick in even when they should.
Deprecating the formatted_... routes nicely
the formatted routes need to warn you, and continue to work with positional arguments
formatted_person_url(1, :xml)
-
DHH November 23rd, 2008 @ 01:17 PM
- Milestone cleared.
I'd really like to see this make it into 2.3. The formatted_ stuff was a hack anyway. Would be great to get rid of it. Anyone working on this have some comments for koz's concerns?
-
aaronbatalion November 24th, 2008 @ 07:41 AM
Attached is a new patch that gets rid of :format's, and answers Koz's concerns.
1) Included a deprecation warn for each use of formatted_url* methods. If someone would like to suggest the proper warning text, I'm sure it can be improved.
2) Fixed the generation optimisations implementation to still work for the previously route.optimise?-able routes.
3) Started to rip out all direct calls to formatted_*, but that might need another pass to completely deprecate it.
-
Repository November 26th, 2008 @ 09:57 AM
- State changed from new to committed
(from [fef6c32afe2276dffa0347e25808a86e7a101af1]) Added optimal formatted routes to rails, deprecating the formatted_* methods, and reducing routes creation by 50% [#1359 state:committed]
Signed-off-by: David Heinemeier Hansson david@loudthinking.com http://github.com/rails/rails/co...
-
Ryan Bigg October 14th, 2010 @ 08:22 AM
- Tag cleared.
- Importance changed from to Low
Automatic cleanup of spam.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 1215 Add :only/:except options to map.resources I've opened a separate ticket, #1359, to discuss dispensi...
- 1215 Add :only/:except options to map.resources @Lourens: You should take a look and get involved in aaro...
- 1359 Add optional :format argument to named routes (from [fef6c32afe2276dffa0347e25808a86e7a101af1]) Added o...