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

feat(http): add headers and methods to the http namespace #4304

Closed

Conversation

halvardssm
Copy link
Contributor

Hi!

I am submitting this PR as I see the usefulness of HTTP methods and headers to be available in this typed and shorthand matter. The headers and methods are crawled from MDN, and thus also linked. I am not sure how to deal with the copyright, so feedback is appreciated in case I did not do this correctly.

Please let me know if anything needs to be changed.

@halvardssm halvardssm requested a review from kt3k as a code owner February 11, 2024 16:42
@github-actions github-actions bot added the http label Feb 11, 2024
@kt3k
Copy link
Member

kt3k commented Feb 12, 2024

We are probably not accepting method.ts as we recently removed a very similar module from std/http #3951. The main reason for it was that we couldn't reach the consensus on what should be included in it.

I'm not sure about header.ts. Does this list include all headers standardized in RFC?

(Feedbacks on this PR from the community are welcome.)

@kt3k kt3k added the feedback welcome We want community's feedback on this issue or PR label Feb 12, 2024
@halvardssm
Copy link
Contributor Author

For the methods, I followed the RFC 9110 list, but after researching a bit more, it seems that the standard methods are maintained at IANA. Is it this list that did not find a consensus? It seems to me that there are two categories here, webdav and standard HTTP. I think it would at least be beneficial to have the standard headers present, but I wouldn't see an issue with having all of them either (we can just split them up).

For headers, I would probably change it to reflect this list at IANA, but separate it by status.

@kt3k
Copy link
Member

kt3k commented Feb 12, 2024

For the methods, I followed the RFC 9110 list, but after researching a bit more, it seems that the standard methods are maintained at IANA. Is it this list that did not find a consensus?

Some preferred to list only RFC 9110 ones. Some preferred RFC 9110 + IANA methods. Some preferred what Node.js lists in require("http").METHODS (which is different from IANA list). Some argued any string is valid http method (which is apparently what browsers doing)... So we gave up to define 'standard' set of HTTP methods

For headers, I would probably change it to reflect this list at IANA, but separate it by status.

Maybe a good idea, but can you split it to another PR? I think header.ts and method.ts require independent discussions.

@halvardssm
Copy link
Contributor Author

Some preferred to list only RFC 9110 ones. Some preferred RFC 9110 + IANA methods. Some preferred what Node.js lists in require("http").METHODS (which is different from IANA list). Some argued any string is valid http method (which is apparently what browsers doing)... So we gave up to define 'standard' set of HTTP methods

Wouldn't this be solved by having two groups? One for RFC 9110 and one for IANA, and then have one export that combines the two (alternatively also have a node headers export for compatibility)? It somehow feels like removing them altogether is not the optimal solution.

Maybe a good idea, but can you split it to another PR? I think header.ts and method.ts require independent discussions.

Sure, Ill submit another PR 💪🏻

@kt3k
Copy link
Member

kt3k commented Feb 13, 2024

Thanks for splitting PRs!

Closing this one in favor of #4317 and #4320

@kt3k kt3k closed this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback welcome We want community's feedback on this issue or PR http
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants