-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix multi-method routes to append '(.:format)' only once #188
Conversation
Because the same path string is used for each loop when the routes are being built, appending the string using `<<` modifies the original, causing multiple '(.:format)' strings to be appended. This fixes the behavior, resulting in the proper path output. Signed-off-by: Evan Owen <kainosnoema@gmail.com>
Thank you. Would you please be so kind to add a line to the CHANGELOG. I'll merge it then. |
Done. I've got another fix coming for some issues I've had mounting Grape APIs at paths like this:
Currently that only seems to work with non-Grape Rack apps. |
Sorry, had the wrong issue number with that first commit. Fixed. |
Yes, that mount problem is #60 I believe, would be great if you fixed it! |
Fix multi-method routes to append '(.:format)' only once
@kainosnoema if you have a moment, could you please confirm that this fixes #60? |
@dblock no, this has nothing to do with #60. @stephencelis and I spent quite a bit of time trying to solve the mounting issue, but got bogged with the odd differences between how Grape treats vanilla Rack apps vs other Grape APIs. To be bluntly honest, the whole experience has dampened our enthusiasm. Due to several serious issues (including #60), working with Grape has been mostly frustrating so far. Instead of building around the strengths of Rack, it seems to add a lot of unnecessary complexity while trying to add functionality. As an experiment, we've rolled our own framework that has most of the same functionality, but more closely follows Rack conventions. It's currently at ~600 lines (vs Grape's 7k+) and so far seems easier to build on and augment. Through the process we've come up with a few ideas on possible improvements, but they're not quite mature yet. I'll keep you posted. |
Thanks @kainosnoema. Would love improvements to Grape or, if Grape has served as a trial by fire of how not to do something, a competing open-source framework that's better. |
Because the same path string is used for each loop when the routes are being built, appending the string using
<<
modifies the original, causing multiple '(.:format)' strings to be appended. This fixes the behavior, resulting in the proper path output.I started with a spec which fails like this before the commit: