Allow passing new Headers() to res.writeHead(...)

[jimmywarting]

What is the problem this feature will solve?

I would like for standard Headers to be passable to http response.writeHead

An example:

http.createServer((req, res) => {
  const headers = new Headers()
  res.writeHead(200, 'OK', headers)
})

What is the feature you are proposing to solve the problem?

Want things that are more web-ish

What alternatives have you considered?

No response

[ShogunPanda]

@nodejs/http What do you think about this? Headers is from fetch API.

cc @marco-ippolito, which is interested in implementing this if we choose to opt-in

[ronag]

SGTM.

[ronag]

Would maybe also be interesting to explore allowing returning a Response from the createServer callback?

[mcollina]

Would maybe also be interesting to explore allowing returning a Response from the createServer callback?

I would be -1 to that. The createServer callback is an event emitted... and adding this would make it exceptionally complicated.

[jimmywarting]

Would maybe also be interesting to explore allowing returning a Response from the createServer callback?

I like that idea also
note that some ppl are using async
http.createServer(async (req, res) => { ... })

So maybe also waiting for something to resolve into a response would be useful.

But then we should probably have respondWith to have little parity with service worker, as to allow offloading some part of your code off the server onto ServiceWorker to allow the app to work offline.

incomingRequest.respondWith(fetch(url))`

Talked a little bit about this here:
#42529 (comment) but the issue got closed

---

b/c the createServer is a event emitted callback then maybe it would be easier to extend it to have respondWith

[mcollina]

I'm -1 as I don't think this will benefit the ecosystem. The Headers object is significantly slower than what we currently do (to the point I recommend against using it whenever possible). Adding this support to a relatively fast API (writeHead) will make the ecosystem worse.

Moreover, Headers is not designed for server usage (cookies, forbidden headers). Note that WinterCG is working on making it more server-friendly.

I think this kind of compatibility would be better left for higher level frameworks.

[mcollina]

I like that idea also
note that some ppl are using async
http.createServer(async (req, res) => { ... })

This is one of the major source of promise issues when people are using our http server APIs. Don't do this.

[jimmywarting]

This is one of the major source of promise issues when people are using our http server APIs. Don't do this.

care to explain why it's an issue to use async?

[bnoordhuis]

It creates promises when they're not needed. Wastes memory and CPU for no reason.

[mcollina]

This is one of the major source of promise issues when people are using our http server APIs. Don't do this.

care to explain why it's an issue to use async?

This video from @jasnell covers it fairly well: https://m.youtube.com/watch?v=XV-u_Ow47s0.

[ShogunPanda]

I'm also -0.5 as I foresee problems as @mcollina already pointed out.
Do we have a consensus to close this?

[mcollina]

I'd be ok in adding a setHeaders() method for example. I just don't want to modify writeHead() or any other low-level APIs.

[ShogunPanda]

I think that's a good compromise.

@marco-ippolito Are still interested in implementing it?

[marco-ippolito]

I think that's a good compromise.

@marco-ippolito Are still interested in implementing it?

Yes 👍🏻

[jimmywarting]

I think that's a good compromise.
@marco-ippolito Are still interested in implementing it?

Yes 👍🏻

if what @ronag suggested in #42529 (comment) becomes available (creating a more ServiceWorker-like server) of having something such as evt.respondWith(new Response(...)) then i don't think i will need a new setHeaders() method.

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);
  })());
});

[marco-ippolito]

We have res.setHeader , we could implement res.setHeaders, with input object or array, basically like writeHead without status code or status message.
WDYT?

[mscdex]

If a .setHeaders() is added that only supports Headers instances, I think that will confuse a lot of people.

[marco-ippolito]

If a .setHeaders() is added that only supports Headers instances, I think that will confuse a lot of people.

Imho it should support Headers, object, array.

[jimmywarting]

On the fence weather or not it should support Object and arrays, All i care about is being able to assign standard Headers i get from another api endpoint.

I maybe could have use something like:

  res.writeHead(200, 'OK', [...headers]) // or
  res.writeHead(200, 'OK', Object.fromEntries(headers))

but don't know if it would work... given some headers like set-cookie that aren't mangled into on single header joined with , or if res.writeHead accepts a 2D array of [[key, value], [key, value]]. I'm unsure about Object.fromEntries, it would only pick the last pair if some key would appear twice (like set-cookie)

maybe: res.writeHead(200, 'OK', [...headers].flat()) works?

---

If it should accept anything else other than Headers (such as Object & arrays) then i think it should only accept the same arguments as the Header constructor can accept as an input.
That means that we would do something like:

res.setHeaders = function (value) {
  const headers = new Headers(value)
  for (const pair of headers) {
    // append headers
  }
}

that way we can accept both arrays, objects and headers along with something that's iterable such as

new Headers(new Map([
  ['content-type', 'text/plain'],
  ['content-length', '1024']
]))

(headers are also iterable and yields an array of tuples - so new Headers(new Headers()) works too)

otherwise i'm just fine with doing:

res.setHeaders( new Headers({foo: 'bar'}) )

---

Speaking of adding a way of setting headers using the Headers class... should there also be a way to get headers as a Headers class too? aka: request.getHeaders() (but that's already taken)

[marco-ippolito]

@jimmywarting setHeaders(Headers|Map) has been merged and will be release probably in v20 #46109
For getHeaders maybe you could open another issue as feature-request and we could close this one wdyt @ShogunPanda

[ShogunPanda]

Yes @marco-ippolito, I agree. @jimmywarting please open a new issue to request the getter.