Implement tower::Layer for tonic_web::Config

[slinkydeveloper]

• Fix Remove NamedService bound for tonic_web::enable #1115
• Fix Implement tower::Layer for tonic_web::Config #1117

[slinkydeveloper]

Perhaps one question @LucioFranco, should we document it somewhere in the rustdocs of the method tonic_web::config or of the struct tonic_web::config::Config that this trait is implemented? Or what rustdoc already generates in the trait implementation section is enough for you?

[LucioFranco]

While I think the idea of adding a layer impl makes sense it feels like the code is actually a bit harder to understand at a glance. So I am inclined to continue to force users through the enabled api. I am curious to hear what @davidpdrsn and @neoeinstein thoughts might be.

[slinkydeveloper]

@LucioFranco in my case, I'm not using tonic::Server and I'm implementating manually the underlying service, reusing tonic only for decoding/encoding the grpc/grpc-web protocol. In this kind of use cases, supporting tower layers is really handy.

Somehow related to this (but I can open another issue to discuss it), I found out that the CORS code in tonic-web is doing more or less the same of https://docs.rs/tower-http/latest/tower_http/cors/index.html. I was wondering if it makes sense to just expose a GrpcWeb layer that transcodes grpc-web to grpc, and let the tower-http cors implementation handle the cors preflight. So one could either pick the "transcoding layer" manually, or can use tonic_web::enable to get the cors layer from tower-http composed with the transcoding layer. WDYT?

[LucioFranco]

@slinkydeveloper that seems like an extremely valid argument for the layer stuff. Maybe we should create a to_layer fn that produces a WebLayer that is a bit more clear of a distinction. I believe this should work for your use case as well?

Yeah, I think that cors idea is great, we originally had it in here before the one in tower-http was a thing, so more than happy to remove code.

Thanks!

[slinkydeveloper]

@LucioFranco to_layer sounds good to me, although I have in mind something different (which involves breaking changes):

• WebLayer is exposed directly, it has no parameters (doesn't need them), has a new method, and it implements Layer, generating GrpcWeb<S> services
• tonic_web::enable composes Cors layer from tonic-http and WebLayer. If a user wants more control over the cors layer (like me), they can just instantiate manually WebLayer and compose it with whatever configured cors layer they want
• Config gets removed, there is no need for that anymore because all the fields are used to implement cors

WDYT?

I can split this work in two or three separate PRs as well (perhaps keeping Config as deprecated?).

[LucioFranco]

Yeah, that sounds like a great plan, smaller separate PRs will make my life much easier! This all sounds good to me. Thank you!

[slinkydeveloper]

@LucioFranco I think this PR as it is should be fine now. I'm working on the second PR to reimplement cors using tower_http::cors

[LucioFranco]

LGTM just one question

[LucioFranco]

perfect! Thanks!