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

Exposing vibe.web.web.getRequestContext() #1937

Closed
BenjaminSchaaf opened this Issue Sep 22, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@BenjaminSchaaf
Contributor

BenjaminSchaaf commented Sep 22, 2017

Exposing vibe.web.web.getRequestContext() would allow users to extend the functionality already present in vibe.web.web. A good example would be a theoretical signIn function:

// How it would have to currently be written/used
void signIn(scope HTTPServerResponse res, User user);

// What this change would allow, fitting in more closely with the declarative style of vibe.web.web
void signIn(User user);

I'm opening this issue before submitting a pull request for some feedback on this change.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Sep 22, 2017

Member

There is just one reason that (kind of) speaks against this, AFAICS, namely that the request/response handling API will probably change in the redesign of the HTTP module - the old API will stay, but will be less efficient. So adding such an API now would be based on a mechanism that is known to be a dead-end. On the other hand, of course request/response parameters are already possible, and it will usually actually help encapsulating the req/res access, so this really is not a very strong argument.

API-wise, I'd expose the request and response objects individually and keep the RequestContext struct private, as that would fit better with the existing language accessor and allows to be a bit more flexible w.r.t. to internal and external representation.

Member

s-ludwig commented Sep 22, 2017

There is just one reason that (kind of) speaks against this, AFAICS, namely that the request/response handling API will probably change in the redesign of the HTTP module - the old API will stay, but will be less efficient. So adding such an API now would be based on a mechanism that is known to be a dead-end. On the other hand, of course request/response parameters are already possible, and it will usually actually help encapsulating the req/res access, so this really is not a very strong argument.

API-wise, I'd expose the request and response objects individually and keep the RequestContext struct private, as that would fit better with the existing language accessor and allows to be a bit more flexible w.r.t. to internal and external representation.

@BenjaminSchaaf

This comment has been minimized.

Show comment
Hide comment
@BenjaminSchaaf

BenjaminSchaaf Sep 25, 2017

Contributor

Closing since PR was merged, thanks!

Contributor

BenjaminSchaaf commented Sep 25, 2017

Closing since PR was merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment