-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
attach req
as res.req
#28673
Comments
If you're that concerned about the number of parameters (growing or otherwise stable), then use an object for everything else besides
Why would you need a framework just to do I don't see the point of this considering how easy this is to do in userland. Besides simply doing |
The point is not that you need a framework to achieve the linking—I know that it's a one liner. This issue is talking about the effects of not having that one liner in core at an ecosystem level. Frameworks have proven its utility. And by not having it, it makes it harder for framework-agnostic utility modules to prosper. Which contributes to the ecosystem-wide lock in of HTTP helpers in framework-specific wrappers.
This doesn't really solve the problem, it just makes it easier to grow over time. All of the confusion/awkwardness around "to |
I like this idea in theory because of “enabling framework-agnostic modules”. I think it’d help to have an example of a real-world situation where this would help. I’d also encourage you to open a PR—you’re likely to get more engagement from collaborators. |
That's totally fair! I'm more than happy to submit a pull request if the Node team signs off on it. Just don't want to spend time to have it shot down, since I haven't contributed to core I'm unsure how much of a time sink it is.
I went into it a bit in the original post, but I can try to point to more... One example is for a set of HTTP helpers. I've wanted to create: getHeader(req, name)
getCharset(req)
getType(req)
...
setHeader(res, name, value)
setAttachment(res, filename)
setAuthentication(res, schemes)
setCharset(res, charset)
setType(req, type)
... This works well for 90% of the functions. All of the But there are a few where the This is unfortunate, since it ruins the consistency of the API. (Even further... these helpers translate very nicely to methods, such that you can use the
Edit: If others have example I'd love to hear them. Anything where the 90% case involves just |
Another way to look at what kinds of use cases are hampered by this is to look at Express's existing
It gets used for checking things like if the method is There are lots of these little edge cases where you wouldn't realize that you need access to A framework-agnostic set of helpers would need it slightly less, since things like cookies configuration can be decoupled. But there are still a significant amount of cases that end up using the |
In what way does Micro's send not currently handle HEAD requests properly? It looks proper to me; Node.js will actually automatically discard the data sent to |
Interesting! That's great to know, I'll strike out that example from above then. I didn't realize that with all the indirection in the core response handling—but that makes sense. Actually, based on the responses to issues/PRs in Micro it sounds like the maintainers of Micro weren't aware of that either. I assume that means that these lines in Express are unnecessary? Either way though... that further reinforces the idea that |
Add `res.req` and `res.redirect()` in http server for convenience. Fixes: nodejs#28673
This change makes it possible for userland http-related modules to pave over edge cases that require referencing the original request when handling a response--making a "Lodash for HTTP" library possible. More information and research in nodejs#28673 Fixes: nodejs#28673
This change makes it possible for userland http-related modules to pave over edge cases that require referencing the original request when handling a response--making a "Lodash for HTTP" library possible. More information and research in #28673 Fixes: #28673 PR-URL: #36505 Reviewed-By: Robert Nagy <ronagy@icloud.com>
This change makes it possible for userland http-related modules to pave over edge cases that require referencing the original request when handling a response--making a "Lodash for HTTP" library possible. More information and research in #28673 Fixes: #28673 PR-URL: #36505 Reviewed-By: Robert Nagy <ronagy@icloud.com>
I'd like to propose attaching the
IncomingMessage
(req
) object in an HTTP server to theServerResponse
(res
) object:Because it is often the case that when creating a response you need to access information related to the request itself.
(If you read on you'll notice that all of the popular frameworks do this under the covers, and that Node's internal logic also uses this connection, but that it's not available for smaller utilities in userland.)
The current state of things.
As things stand right now, for HTTP response-related logic you often end up needing to have access to the
req
object too to handle a variety of edge-case situations, which leads to either verbosity or inconsistency, and either way… confusion.For example, imagine writing a
sendJson
helper:All seems fine. But this doesn't account for
HEAD
requests that should not include a body. To fix this you'd have to add(req, ...)
to the signature, which is unintuitive. This is a frequent issue with HTTP-related utilities, and it part of the reason why you end up having to reach for a framework, instead of seeing a proliferation of small, useful HTTP functions.All the major userland frameworks do this already.
This has already been established as a common practice in userland, because of how often you need to tweak a response based on the request.
res.req
.reply.request
.h.request
andresponse.request
.response.request
.res.req
.Often times the
req
is needed only for HTTP-specific plumbing edge cases (like checkingHEAD
orOPTIONS
requests) that are not immediately obvious to the user's mental model. Which is why it's helpful if HTTP logic can access it implicitly for the edge cases where it's required.Most of these frameworks also do the reverse, attaching the response to the request for parallelism/consistency.
Technically, you can already do the reverse.
You can actually already do the reverse—accessing the
res
object from thereq
without a framework, by using thereq.connection._httpMessage
property. Now, this isn't exactly encouraged seeing as it's an undocumented property, but it shows that it's not out of the question to create these kinds of links between these two objects.Why not always pass
req
into helpers?A counter-argument would be to say that
req
should just be passed into any helper that needs to use properties from it. This sounds reasonable at first, but the issue with this approach is that it leads to mismatch with user expectations.The solutions become inconsistent because access to the
req
isn't always required. For example, consider a few different helpers for setting the correct headers on a response:setHeader(res, name, value)
helper doesn't need thereq
, because it just sets the value to whatever was passed in.setCors(res, options)
helper would actually need to besetCors(req, res, options)
because you need to determine if the request used theOPTIONS
method.setAuthenciate(res, scheme, options)
helper also doesn't need thereq
object.setRequestId(res, options)
would need thereq
to default the ID in case it was already set.The exact nature of the examples is irrelevant—they could be implemented in many different ways. But the point is that access to
req
is required in certain situations when building a response, but not in others. Which leads to APIs that are inconsistent in signature between(res, ...)
and(req, res, ...)
, causing confusion.And it's often required for low-level, edge-case HTTP behaviors that most users of these modules ideally don't need to think about, because they are HTTP plumbing.
You might argue that then all response-related helpers should include
(req, res, ...)
to keep it consistent. But if you consider things likesendJson(res, status, json, options)
, adding an extra(req, ...)
to every arguments list becomes tedious. Especially so for helpers that don't use thereq
at all, and are just forced to add it for consistency for others. It feels pretty dumb to have to dogetBasicAuth(req, res)
just because the library needs consistency to handle ~10–20% cases.This awkwardness is why the major HTTP frameworks for Node have all included a way to access the request from the response themselves. They know that for users it's much simpler to think about working on either the request or the response. But that edge cases require having access to both objects at once.
Node's core uses the "reference" itself internally.
When creating the
res
object in the first place, Node's core has access toreq
. And it actually does some of the same exact things that are being advocated for here, by calculating some internal logic for the response based on the request.node/lib/_http_server.js
Line 148 in a013aa0
node/lib/_http_server.js
Lines 155 to 156 in a013aa0
node/lib/_http_server.js
Lines 713 to 716 in a013aa0
These are all internal situations where specific response-related logic is being calculated by the request metadata and attached to
res
for future use. It just happens that Node already has thereq
in scope when creating theres
. But it would be nice for userland to be able to do the same sorts of things with theres
only.This would open up new userland abilities.
Right now HTTP-related pieces of userland are almost exclusively handled by large monolithic frameworks like Express or Hapi. This is partly because there doesn't exist a simpler, Lodash-like set of utilities for handling HTTP requests in a simple functional way. Being able to link
req
andres
would make building an intuitive library like this much easier.Not only does this allow for people to opt-out of using a framework. But, more importantly, it allows other utilities to be built in a framework-agnostic way.
You could imagine…
…a world where the frameworks themselves aren't necessary. This would be helpful Lambda-like situations, or for simply adding extra HTTP-aware logic to adjacent domains, or just for keeping things extremely simple while prototyping. A lot of different use cases would benefit.
A good example of this is Vercel's built-in HTTP modifications. These kinds of behaviors are happening all over the place in the FaaS world, and only further fragment Node's HTTP handling. It would be amazing if non-monolithic solutions to this problem could thrive.
In summary...
Attaching
res.req
is a simple addition to Node's core that would make writing HTTP servers without needing a framework easier, which is especially helpful for writing framework-agnostic utilities, or in Lambda-like environments where you need smaller dependencies, or in performance-critical situations where framework overhead is harmful. All major userland HTTP frameworks already all do this.It would also make it possible for userland to step up and create a nice set of framework-agnostic helper modules (eg.
setCors
, orsetRequestId
), without forcing them to have more confusing APIs than their framework-coupled counterparts—since at the moment they can't benefit from the link.I hope that all makes sense. I think this would a small change, but one that unlocks a lot of value in userland at many layers of the stack.
Thanks for reading!
The text was updated successfully, but these errors were encountered: