actix-net icon indicating copy to clipboard operation
actix-net copied to clipboard

impl Transform for Option

Open KoltesDigital opened this issue 2 years ago • 1 comments

This topic continues https://github.com/actix/actix-web/pull/2858 , whole goal is to .wrap an optional middleware. Condition middleware exists but requires a transformer even when unused. It seems to me that using an Option is more natural. But as that PR was on a different crate than Transform's, a newtype was introduced, and .into() required an explicit annotation. Sadly this was non-optimal.

@fakeshadow suggested that this feature should rather be moved into Transform's crate, so that .wrap can natively take an Option<SomeMiddleware>. This would mean moving the ConditionMiddleware code too.

KoltesDigital avatar Sep 01 '22 18:09 KoltesDigital

This is a good pattern for basic conditional middleware consider how boolean combinator can be used in recent rust:

service.wrap(condition.then_some(Middleware)) // default optional.
service.wrap(condition.then(|| Middleware)) // lazy optional.

That said advanced (specialized typed) condition middleware is still needed. If the middleware end up mutate input/output types then actix-service would not be aware of it as no concrete types can be used inside the crate.

fakeshadow avatar Sep 02 '22 11:09 fakeshadow