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

utilize reserved http-core crate for primitives #514

Closed
robjtede opened this issue Dec 1, 2021 · 6 comments
Closed

utilize reserved http-core crate for primitives #514

robjtede opened this issue Dec 1, 2021 · 6 comments

Comments

@robjtede
Copy link

robjtede commented Dec 1, 2021

Proposal

I noticed you've reserved the http-core crate.
Would there be interest in moving some of the primitives from http down to it?

I'm thinking:

  • definitely:
    • Method
    • Version
    • StatusCode
  • possibly:
    • Uri
    • HeaderName
    • HeaderValue

Motivation

These are the types we use and re-export from http in Actix Web.

@olix0r
Copy link

olix0r commented Dec 1, 2021

How does splitting this crate up help? What parts of http are you trying to avoid?

@robjtede
Copy link
Author

robjtede commented Dec 1, 2021

How does splitting this crate up help?

A crate with just primitives could reach v1.0 much more quickly, providing confidence to its users. Also would allow less frequent breaking changes for these types in the face a desire to use newer rustc features (think const panic in HeaderName::from_static) causing raised MSRVs and, therefore, a semver-incompatible release.

What parts of http are you trying to avoid?

Basically the opinionated parts.

For example, parts of HeaderMap require the fnv crate; which is opinionated. We've chosen aHash for our map types so we would choose not to carry this dep if possible. Same goes for Request/Response, we've got some optimisations around request head re-use that is just not possible with this lib so it's dead code for us.

The parts of http we want to use are either simple or already considered best-in-class. These are the types which could benefit from a "core" approach, imo.

@seanmonstar
Copy link
Member

parts of HeaderMap require the fnv crate; which is opinionated.

The fnv crate is tiny. If anyone were motivated, we could very easily just inline it, it's less than 10 lines.

Same goes for Request/Response, we've got some optimisations around request head re-use that is just not possible with this lib so it's dead code for us.

Is it really not possible to do sufficient optimizations with Request/Response? When I was playing the benchmark game with hyper, I was able to reuse some pieces that made it "win" back then. I think it would be beneficial for libraries to be able to consider a single type for an http::Request or http::Response.

It could be interesting if we could provide trait Request, trait Response, and trait Headers, but I believe the way generics work in Rust would just make it a pain for end users.

Besides that, I think the http crate currently does its purpose well: be a small crate of just HTTP primitives. Splitting it doesn't seem to provide much value to end users, from what I have gathered.

@robjtede
Copy link
Author

My examples of why we chose to roll our own types don't really illustrate the point of this issue in particular.

be a small crate of just HTTP primitives

If you look at the definition of "primitive" from a lang perspective, then they are building blocks for larger structures, right? Using that, we can say:

  • HeaderMap is not a primitive in the same way that a HashMap is not a primitive;
  • Request and Response are further composed of both primitives and non-primitives (request line / status line + header map);
  • Extensions isn't a HTTP concept at all.

@robjtede
Copy link
Author

If you don't think you'll use it, would you consider transferring ownership of the http-core crate?

@robjtede
Copy link
Author

Some amount of no_std support could be incrementally added using this reserved crate. See #551.

@robjtede robjtede closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants