Skip to content
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

Open
ikitommi opened this issue May 22, 2018 · 6 comments
Open

Make :headers optional in the Ring SPEC Response #328

ikitommi opened this issue May 22, 2018 · 6 comments
Labels

Comments

@ikitommi
Copy link
Contributor

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 and ring-spec, we get failures for this.

Related: metosin/reitit#83 (comment)

@weavejester
Copy link
Member

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.

@ikitommi
Copy link
Contributor Author

Sounds legit. Is there a plan for 2.0? Happy to help, is possible.

@weavejester
Copy link
Member

I haven't written up a formal draft yet, but in a nutshell I expect to add namespaced keywords (like :ring.request/headers) and better async support for requests. The spec will be incompatible with the 1.x specs, but because it uses different headers, we can test for it and support both 1.x and 2.x in middleware.

@ikitommi
Copy link
Contributor Author

ikitommi commented May 27, 2018

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.

@weavejester
Copy link
Member

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 (s/keys)

@KingMob
Copy link
Contributor

KingMob commented Dec 8, 2024

If/when Ring incorporates support for HTTP/2+ , this issue might be a moot point, because all responses will have the :status pseudo-header.

You could remove the pseudo-headers to try and get an empty :headers map again, but I wouldn't recommend it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants