Skip to content

Conversation

@benmccann
Copy link
Member

@benmccann benmccann commented Mar 17, 2021

This isn't working yet, but I thought I'd share for feedback on the direction

I introduced a Headers class as it was too annoying to deal with values that could be string|string[] everywhere and converting between all the different headers types

@Rich-Harris
Copy link
Member

I wonder if we're overcomplicating this? According to the 'message.headers' section of https://nodejs.org/api/http.html#http_class_http_incomingmessage, the only time a header could be an array of strings is set-cookie. And the only time set-cookie could be involved is when prepare or an endpoint is populating response headers (it doesn't make sense in the context of request headers, even if the type definitions don't distinguish between request and response headers).

In other words I think we might be able to add

type RequestHeaders = {
  [key: string]: string
};

type ResponseHeaders = {
  'set-cookie': string | string[];
  [key: string]: string;
};

and call it a day. We'd need some logic for merging headers (using the logic articulated in the link above) but as far as the types go I think we can probably make this quite simple.

Looking into this now

@benmccann
Copy link
Member Author

Closing in favor of #445

@benmccann benmccann closed this Mar 18, 2021
@Conduitry Conduitry deleted the headers branch April 1, 2021 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants