The problem with the mock

[iliakan]

1. There's a need for req, connection property addressed by PR Update request.js #18
2. There's a need to keep other properties bound to req when passed by passport. Maybe a foreach loop or an additional configuration will do.

The use case: I have req.log method which logs with request information (so I know the context). I also use passReqToCallback: true in strategies to have the req object (to log it and to merge profiles).

With the new koa-passport, req.log is undefined, so the project dies.

[rkusa]

I've merged #18, thanks!

Is req.log some common used method - or very specific for your application? I think req.req.log should work (though being somehow ugly)?

[iliakan]

req.log is kind of widely used method name for logging req-specific things, but it is not enforced by a framework. req.req is ugly, true, but I think that'll do for me.

Could we create a mock request as Object.create(request) or Object.create(req)? So that it naturally inherits request properties. That would solve this kind of problems.

[rkusa]

I like your idea of using req itself as the base when creating the mock. Would you mind testing if the last commit works for you?

[iliakan]

It must be req, not request, right?

[rkusa]

I could use request as the base, too, but the commit uses req.

[iliakan]

It works for me.

Do I get it right that passport.js uses your mock req everywhere, including the callback (passReqToCallback: true)?

[rkusa]

Thanks for testing!

Do I get it right that passport.js uses your mock req everywhere, including the callback (passReqToCallback: true)?

Yes

[iliakan]

Koa generally keeps req virgin and modifies it's own request object.

I'd suggest to inherit from request instead.

[rkusa]

I am fine with inheriting from request instead of req. But than, you've to move your log method to request.

In general, the decision is: where to expect user added properties/method. In the request or the req object.

[iliakan]

I submitted the PR to turn to request.
That follows Koa conventions and is generally convenient. I changed my property to use request as well. Using req previously was actually a hack to make it work, now it would be better.

[iliakan]

Please close the issue when you publish the new version, I'll replace my branch with it.

[mfornasa]

@iliakan could you provide a working mocking sample?

[iliakan]

@mfornasa What you mean?