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

impl Transform for Option #465

Open
KoltesDigital opened this issue Sep 1, 2022 · 1 comment
Open

impl Transform for Option #465

KoltesDigital opened this issue Sep 1, 2022 · 1 comment

Comments

@KoltesDigital
Copy link

KoltesDigital commented Sep 1, 2022

This topic continues actix/actix-web#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.

@fakeshadow
Copy link
Contributor

fakeshadow commented Sep 2, 2022

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.

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

2 participants