-
Notifications
You must be signed in to change notification settings - Fork 520
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
Make :headers optional in the Ring SPEC Response #328
Comments
I think this is reasonable, since Clojure tends to encourage maps that can omit keys when the information isn't available. However, as adapters currently assume the headers are a map, it might be a good idea to push this change forward into the 2.0 spec. |
Sounds legit. Is there a plan for 2.0? Happy to help, is possible. |
I haven't written up a formal draft yet, but in a nutshell I expect to add namespaced keywords (like |
Interesting. Unrelated to the issue, but I'm currently not that happy with the "namespacing all the things" movement, more code to type, spec (and schema) support unqualified keys and in many occasions, the data doesn't leave it's data handlers, e.g. requests are handled only by the request handlers. But that's just me - today. Looking forward to seeing the new draft when it starts to take form. |
With namespace aliasing and the syntax in Clojure 1.9, it's not too much more verbose: (require '[ring.request :as req])
#::req{:status 200, :headers {}, :body "Hello World"} And it has the advantage of making the data unambiguous, even when the data is incomplete: #:ring.request{:status 201}
;; vs
{:status 201} It also means we can set up schema more concisely, as we only need to specify |
Setting
:headers
is mandatory in the Ring SPEC for responses. As it is effectively the same as not setting the:headers
at all (which is both less code & faster), I propose it's made optional. This would not be even a breaking change?In the era of
clojure.spec
andring-spec
, we get failures for this.Related: metosin/reitit#83 (comment)
The text was updated successfully, but these errors were encountered: