-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Normalize user-provided HttpLink headers by lower-casing their names. #8449
Conversation
lgtm |
): typeof headers { | ||
if (headers) { | ||
const normalized = Object.create(null); | ||
Object.keys(Object(headers)).forEach(name => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why you chose the forEach approach instead of return reduce
. In this day and age, everyone is liking their immutability very much, even in such trivial cases. Is it just preference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sometimes do use reduce
, but reduce
would still require mutating the normalized
object to accumulate the properties if you don't want to shallow-clone the object every time you add a single property (which wastes memory and time). From that perspective, a forEach
loop is faster and simpler to read than a (naive copying) reduce
loop. No need to guess which callback parameter is which!
On a more general note, I like immutability, but I like it best when it doesn't require me to allocate and throw away a ton of immutable objects along the way to the final object, when simple mutation of would work and be safe, because no one else has seen this object reference yet.
Should fix #8447, by preferring the last (according to
Object.keys(headers)
) header among multiple headers whose names are case-insensitively equivalent.I'm a little worried this could be a user-visible change in some (HTTP-specification-violating) situations. If so, we can easily retarget this PR to the
release-3.4
branch, and make a clear note of it inCHANGELOG.md
.