feat(http): add iana methods to the http namespace
Adds IANA Methods as a follow-up from #4304
I'm -1 because the method can actually be any string. So this PR looks more like a documentation than something worth being in code, especially splitting by norms. I think the name "Rfc9110" should not be in the implementation, but in comments at most. To users, it shouldn't matter where a standard is defined, just like it doesn't matter where and when fetch() was specified.
I'm -1 on this change also. We previously had a methods implementation in std/HTTP but removed it because it was barely used (see #3792). Even if I were for the addition, I'd much prefer the style previously used (see https://github.com/denoland/deno_std/blob/207f5cad67150e2bf0b8a169e33f7130b4ebb9c3/http/unstable_method.ts).
To address the comments:
@Leokuma As this is not specifically typed or provided anywhere else as an enum or variables, having it available in the standard library would be of benefit. As discussed in #4304, there was disagreement on which standards to follow, so to satisfy all camps I separated them by standards, and still provided a collection at the bottom with all the methods from the standards (see the exported HttpMethod). I agree that it shouldn't matter, where the standard is specified, but I would see the benefit of having the methods like this, rather than not at all. Please let me know if you have a different suggestion as to how we can specify the methods.
@iuioiua Although it was barely used, it was used, and as there is generally a standard around the methods, the standard library should in my opinion be a place to specify this for utility purposes. As mentioned in #4304, there was not a consensus on the methods to include, and therefore I decided to split it like this. By using this style, there is also more information regarding the origin of the method, which rfc it is related to, and where to find more information regarding its usage.
To summarise the reason for this contribution (all of the points are my own opinions):
The methods are generally standardized, but allow for any string to be used. By providing the common standards as well as a collected export, we will cater to the different usages (most). It is in my opinion better to provide this, even if it is not heavily used, than not at all.
Feedback on how we can improve on my PR is appreciated!
Can you provide an example use case just so we can see how you envision this being used? I'm guessing it would be used to type routers or something?
@Leokuma At the top of my head, I can think of two use cases:
In fetch
fetch("example.com",{method: HttpMethod.Post})
In deno serve, router or lambda
Deno.serve(({method})=>{
if(method === HttpMethod.Get){
// do something
}
})
Why not just use "GET"? Genuine question. One argument would be correctness. But I have yet to see any examples of this being a problem.
I get what you mean, so in practice, nothing is stopping anyone from using "GET". It is just an accessible helper const/type for those that need it, or want extra typings support. It can also be used for typing a switch, or restrict input for helper functions etc.
Can you clarify what tests you envision? Since it's a const, it's not much to test in regards to functionality and I am not sure testing the values gives much either, no?
Tests can suffice being simple/light. It's just to ensure the plumbing works and will continue to work in the future. PTAL at the tests in http/status_test.ts.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 96.39%. Comparing base (
fdadd53) to head (19ae628).
Additional details and impacted files
@@ Coverage Diff @@
## main #4320 +/- ##
==========================================
+ Coverage 96.38% 96.39% +0.01%
==========================================
Files 466 467 +1
Lines 37588 37712 +124
Branches 5542 5542
==========================================
+ Hits 36228 36352 +124
Misses 1318 1318
Partials 42 42
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@iuioiua Done! Let me know if there is anything else!