Skip to content
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

Do not lowercasing request headers #105

Closed
rohmanhm opened this issue Mar 6, 2019 · 4 comments

Comments

Projects
None yet
4 participants
@rohmanhm
Copy link

commented Mar 6, 2019

Here's my code extend ky instance.

  ky.extend({
    prefixUrl: API_BASE_URL,
    headers: {
      Authorization: `Bearer ${token}`
    }
  })

Here's what i've got on network tab request header.
image

Why did you lowercasing my header key?

@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Mar 6, 2019

HTTP headers are case insensitive.

@rohmanhm

This comment has been minimized.

Copy link
Author

commented Mar 6, 2019

Yeah but just let the user choose how they're gonna write their headers.
Don't force them to use lowercased headers.

IMHO

@szmarczak

This comment has been minimized.

Copy link
Collaborator

commented Mar 6, 2019

Yeah but just let the user choose how they're gonna write their headers.

Is there any reason why? According to the spec it doesn't matter.

@sholladay

This comment has been minimized.

Copy link
Collaborator

commented Mar 6, 2019

I mostly agree with you in principle @rohmanhm. But doing this would actually add some complexity to Ky and I don't think it's worth it given what the spec says.

The reason for the added complexity is that Ky itself is not explicitly lowercasing headers. It's merely a side effect of using new Headers(). Since that's a browser API that we don't control, we'd have to avoid using it to achieve your desired result. I tried removing our usage of Headers in PR #74 (for different reasons) but we ended up keeping it because it turned out to be kind of messy. Also see the discussion here:

That implementation could be made much simpler if we were willing to drop support for a Headers instance in our headers option, but I'm not sure that is the right approach. So unless you can come up with something simpler, I doubt we're going to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.