-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
[http.IncomingMessage] req.toWeb()
#42529
Comments
Doesn't the alternative you propose already work? |
it's too verbose and having to do it in every project. would have been nice to have that built in doe... there is also some signaling stuff, url construction and referrer that i would to set up also to get a complete Request object. const app = http.createServer((r, res) => {
const ctrl = new AbortController()
const headers = new Headers(r.headers)
const url = `http://${headers.get('host')}${r.url}`
req.once('aborted', () => ctrl.abort())
const req = new Request(url, {
headers,
method: r.method,
body: r,
signal: ctrl.signal,
referrer: headers.get(referrer)
})
}) i just thought this a alternative could be to have a getter function like: class IncomingMessage {
get request () {
return this._request ??= new Request(...))
}
}
// and then later be able to do something like:
const app = http.createServer(async (x, res) => {
const json = await x.request.json()
}) on another note it would also be nice to have a class IncomingMessage {
get request () { ... }
respondWith (x) { ... }
}
// and then later be able to do something like:
const app = http.createServer(async evt => {
const json = await evt.request.json()
evt.respondWith(fetch('https://httpbin.org/get'))
}) |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment. For more information on how the project manages feature requests, please consult the feature request management document. |
What about just introducing a new api? To avoid confusion and overlap, i.e.: await http.serve(async req => {
req.respondWith(fetch('https://httpbin.org/get'))
}, { port }) https://deno.land/[email protected]/examples/http_server#using-the-stdhttp-library |
I think the biggest challenge is implementing |
more like: http.serve(async extendableEvent => {
const fd = await extendableEvent.request.formData()
extendableEvent.respondWith(
fetch('https://httpbin.org/post', { method: 'post', body: fd })
)
}, { port }) |
Actually my previous example was wrong. Should be: await http.serve(async req => {
return fetch('https://httpbin.org/get')
}, { port })
`` |
@jimmywarting where does the |
Maybe this should be moved to wintercg? |
FetchEvent is class that extend extendableEvent
think you meant respondWith would like to have something that is more towards something already web-spec'ed like the |
I like it! |
If we take the MDN example and serverify it: const server = http.listen({ port })
server.addEventListener('fetch', (event) => {
// Prevent the default, and handle the request ourselves.
event.respondWith((async () => {
// Try to get the response from a cache.
const cachedResponse = await caches.match(event.request);
// Return it if we found one.
if (cachedResponse) return cachedResponse;
// If we didn't find a match in the cache, use the network.
return fetch(event.request);
})());
}); |
Like that idea even more. |
speaking of the deno server you linked to... that server is just an higher level api on top of what deno already have at it's core so if you are creating a server without any dependencies, then it's more like a fetch event. // Start listening on port 8080 of localhost.
const server = Deno.listen({ port: 8080 });
console.log(`HTTP webserver running. Access it at: http://localhost:8080/`);
// Connections to the server will be yielded up as an async iterable.
for await (const conn of server) {
// In order to not be blocking, we need to handle each connection individually
// without awaiting the function
serveHttp(conn);
}
async function serveHttp(conn: Deno.Conn) {
// This "upgrades" a network connection into an HTTP connection.
const httpConn = Deno.serveHttp(conn);
// Each request sent over the HTTP connection will be yielded as an async
// iterator from the HTTP connection.
for await (const requestEvent of httpConn) {
// The native HTTP server uses the web standard `Request` and `Response`
// objects.
const body = `Your user-agent is:\n\n${
requestEvent.request.headers.get("user-agent") ?? "Unknown"
}`;
// The requestEvent's `.respondWith()` method is how we send the response
// back to the client.
requestEvent.respondWith( // <---
new Response(body, {
status: 200,
}),
);
}
} not entirely like web-speced api's as your suggestion const server = http.serve(port)
server.addEventListener('fetch', (event) => { |
A more web-standards based http server API is a good idea, but I think it's a separate topic to this issue. Converting to a web request is a feature that stands along when for example integrating old node.js code with newer fetch-based stuff. Also a new server HTTP API can be done in userland, whereas a built in method on IncomingMessage cannot. |
Just leaving this here: Astro took this from SvelteKit. Just requires you to install set-cookie-parser: npm i set-cookie-parser && npm i -D @types/set-cookie-parser Then you get a Web Fetch API You can also set a |
👆 Chiming in from the Astro team, we have quite a bit of code that has to manage converting Node's HTTP message primitives to their standardized web equivalents. Modern frameworks have clearly embraced web standard |
I wanted to use Web standard import { serve } from "@hono/node-server";
serve({
async fetch(request) {
return new Response("👋");
},
port: 4000
}); Seems to be working well so far. Would be great to see Node provide an http server API that uses Web standards, similar to Bun. |
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the
never-stale
|
it would be great to have this nativly available in node. (How) can us simpletons help move this forward? |
We on the SvelteKit team would love to be able to get rid of this code, though if it came down to a choice between Both things can be done in userland (converting Node to Web, and designing a standards-based API), but at least for frameworks, the only reason to convert is so that we can provide the standards-based API. If we could deal with |
Chiming in. Next.js user here. I love building third party libraries for Next.js. EXCEPT if I need to support both API Routes from the Pages router and API routes from the App router, both which have their strengths. I would love for these libraries to simply take "req" and be done with it, but we have NextApiRequest (IncomingMessage) and NextRequest (Request). So yes, req.toWeb() would be sick. |
I installed set-cookie-parser but I'm not seeing get_request anywhere |
This was the ticket for me. Definitely agree with others that it would be great to have this built into Node like Deno.serve/Bun.serve. What would be even better is if there was a WinterCG function that could be called the same in all runtimes. |
Server.handle() expects standard `Request` objects and returns `Response` objects. For Node.js you'll need some way to convert to and from those types. We're using @hono/node-server here to provide an API similar to Deno.serve or Bun.serve. See this discussion: nodejs/node#42529
I studied several implementations of this pattern and developed a standalone lib that works with Server instances from |
Do you have a link to the current code? |
Here's a great package for it I found from Micheal Jackson on the Remix team in case anyone is interested: https://github.com/mjackson/remix-the-web/tree/main/packages/node-fetch-server |
This looks more fit-to-purpose. Also significantly smaller and thus hopefully easier to fork if necessary: du -h node_modules/@hono/node-server/
20K node_modules/@hono/node-server/dist/utils/response
40K node_modules/@hono/node-server/dist/utils
324K node_modules/@hono/node-server/dist
340K node_modules/@hono/node-server
du -h node_modules/@mjackson/node-fetch-server/
32K node_modules/@mjackson/node-fetch-server/dist
52K node_modules/@mjackson/node-fetch-server/ Thanks! |
@anderspitman I just extracted it into a public repo - https://github.com/eugene1g/node-http-to-fetch. It's for HTTP1, and HTTP2, has tests, and benchmarks. I should shape up the repo to make this library easier to consume. |
@eugene1g nice! What advantage does this have over node-fetch-server? |
@anderspitman They are similar - both are ~300 LOCs with zero dependencies, expose low-level helpers, and have identical performance, but mine has support for http2 servers via the same interface. |
@eugene1g thanks! |
What is the problem this feature will solve?
Reading the body out of the
http.IncomingMessage
is troublesome without any 3th party package...Request
on the other hand has features such as text, blob, json, arrayBuffer, formData and body for parsing it.What is the feature you are proposing to solve the problem?
It would be a convenient to have something like a
req.toWeb()
utility added onto the simple http server 🚀something like the stream.Readable.toWeb() but for the http.IncomingRequest to web Request
What alternatives have you considered?
currently doing some simplier variant like this one:
having this
req.toWeb()
built in would be very handyThe text was updated successfully, but these errors were encountered: