-
-
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
Don't use .format
suffix in paths if you only have one format
#809
Conversation
I like this. It needs a CHANGELOG entry and possibly a README update around format selection. |
Updated the docs. Do they all seem right? |
Before I merge this... Don't you think it's a little strange that 1 format = no automatic suffix, multiple formats = automatic suffix? What if |
So if I understand you right... class MyAPI < Grape::API
default_format :json
get :hello do { hello: 'world' } end
end As it is: Are you saying that if It makes sense to me that 1 format = no automatic suffix, because there's nothing the suffix would do - you have no choice but to have the API return JSON (short of |
Ok, I completely agree with you, thanks. Merging. |
Don't use `.format` suffix in paths if you only have one format
@ajvondrak: This needs an entry to UPGRADING, please. Before you had |
Can you make a new PR for that please? |
I just tried to upgrade one of our apps from Grape 0.9.0 to ~> 0.10.0. It took me hours to debug the issue as to why Changing
actually fixes the problem. Now, with that change the following URLs will work:
and both will return the same. To be honest, that's what I'd expect when using |
I had a similar sentiment a few times. I would be open to undoing this change carefully as you describe, if you want to try a PR. |
Alright, though I'll probably aim for several PRs as a lot of the things I've described are actually different/non-related things. |
For example, say you have the following API:
If you hit
/hello
, you'd get back the JSON{"hello":"world"}
. This makes sense: you specifically say that your format is JSON.However, before this commit, you could also hit
/hello.xml
, which would confusingly return the JSON{"hello":"world"}
. Why bother exposing this path as a valid API endpoint when we're just going to ignore the format anyway?In this commit,
/hello.xml
(or/hello.json
or/hello.foobar
) will all be 404s. We don't even entertain the notion of an optional format path parameter.Old behavior is still preserved if you add different content types after your
format :xxx
declaration. So, if you have the following API:then
/hello
,/hello.json
,/hello.xml
, and/hello.foobar
all return the JSON{"hello":"world"}
. I don't know why you'd want this behavior either, but it was being enforced by the specs, so I preserved it.What do you think?