seanmonstar/warp

Rationale behind wrapper filters for methods?

Closed this issue · 5 comments

I'm curious about why we write

warp::get(
    warp::path("a")
       .and(warp::path("b"))
);

instead of

warp::get()
    .and(warp::path("a"))
    .and(warp::path("b"))
;

Yea, this inconsistency is a little annoying, I agree. The method filters were some of the first ones I wrote, and back then, I hadn't figured out a nice solution to ditching the (), so anding them caused this nastiness:

warp::get()
    .and(warp::path("hello"))
    .map(|((), ())| foobar())

It's probably clearer if they just worked the same as all other filters, huh? I knew I'd forget to fix something before releasing 0.1...

Ah! That makes sense. I still haven't looked into the "magic" you're doing in here, mostly just playing around to see what the library is like.

The original title of this issue was going to be about the implementation escaping in the return types of those filters as well, but then I realised they were simply "different". That's probably also something worth fixing at the same time.

What's your position on breaking changes? Do you have a rough timeline for a 0.2 that would batch things like this?

The implementation "escaping" was actually because since those functions take a Filter argument, whether that filter was Clone or Copy was lost if the the return value is just impl Filter. By returning concrete types, those qualities still exist thanks to derive(Copy, Clone) on the combinators. Since the combinators aren't publicly exported, and thus not nameable, I'd like to think that the return type here can change, as long as the returned type still fulfills the same traits.

I'll probably hold off on a breaking change for at least a little bit, so that these papercuts can be grouped up. Fixing these filters could probably be done by putting the new ones in warp::method::v2 or something...

Ah I didn't mean impl Filter, I just meant a per-filter newtype struct that could still have derive(Copy, Clone) on it. This would mean a boilerplate impl Filter for each one, but at least you're guaranteed the type names are not changing. The unnameablity is something I didn't consider, 🤔...

Newtypes here can't be done easily, since they take advantage of the combinators and closures. I suppose they could stop using those, but may not be worth the hassle.