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

Export actix_web::dev::ServiceFactory #2302

Closed
wants to merge 2 commits into from

Conversation

chinedufn
Copy link

@chinedufn chinedufn commented Jul 7, 2021

PR Type

Re-export

PR Checklist

  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

Enables:

pub fn create_app() -> App<
    impl ServiceFactory<
        ServiceRequest,
        Response = ServiceResponse<AnyBody>,
        Config = (),
        InitError = (),
        Error = Error,
    >,
    AnyBody,
> {
    App::new()
        .wrap(Compat::new(Logger::default())
        .route("/", web::to(|| async { "hello" }))
}

Related: #2301

Enables:

```
pub fn create_app() -> App<
    impl ServiceFactory<
        ServiceRequest,
        Response = ServiceResponse<AnyBody>,
        Config = (),
        InitError = (),
        Error = Error,
    >,
    AnyBody,
> {
    App::new()
        .wrap(Compat::new(Logger::default())
        .route("/", web::to(|| async { "hello" }))
}
```
@chinedufn
Copy link
Author

The last thing that I still can't do:

fn create_request(body: MyType) -> Request<PayloadStream> {
    TestRequest::post()
        .uri("/index")
        .set_json(&body)
        .to_request()
}

This is useful when I have multiple tests all testing the same route.

Not a huge deal as I can just -> TestRequest, but I'm curious as to why re-exporting Request is an issue? It seems like it's public in actix-http so anyone could just depend on that if they really wanted it?

Still new to actix-web so apologies if I'm missing something.

Thanks!

@robjtede robjtede added A-web project: actix-web B-semver-patch labels Jul 12, 2021
@chinedufn
Copy link
Author

Mmm actually I'm not sure if I can just use TestRequest to handle all of my cases.

Here's a situation where I (think?) I need Request.

/// The type returned from actix_web::test::init_service
pub type TestAppService = impl actix_web::dev::Service<
    actix_web::Request,
    Response = ServiceResponse,
    Error = actix_web::Error,
>;

pub async fn create_app_test_service_with_pool(pool: Pool) -> TestAppService {
    actix_web::test::init_service(api_test_app(pool)).await
}

Let me know if I should open a new issue for this instead of roping it into this PR.

Request is exported by actix-http so I guess if it's decided that it can't be re-exported by actix-web I can just depend on actix-http.

I don't know enough about these types to know that's supposed to be private and what isn't.

@robjtede
Copy link
Member

I'd like to hold off on re-exporting Request for now. You should import it through actix_http.

@robjtede robjtede mentioned this pull request Jul 12, 2021
3 tasks
@robjtede robjtede closed this Jul 12, 2021
@chinedufn chinedufn deleted the expose-types branch July 12, 2021 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants