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

Accept array in Headers constructor #485

Merged
merged 1 commit into from
Feb 22, 2017

Conversation

tschaub
Copy link
Contributor

@tschaub tschaub commented Feb 22, 2017

From the spec, it looks like the Headers constructor should accept an array of arrays.

I've adapted the example for the tests:

var meta = { "Content-Type": "text/xml", "Breaking-Bad": "<3" }
new Headers(meta)

// The above is equivalent to
var meta = [
  [ "Content-Type", "text/xml" ],
  [ "Breaking-Bad", "<3" ]
]
new Headers(meta)

This came up for me as an issue using stream-http in Edge with the Financial Times polyfill. I've opened jhiesey/stream-http#71 and polyfillpolyfill/polyfill-service#1091 in case this can be addressed elsewhere, but it looks to me like this might be the right fix.

@dgraham
Copy link
Contributor

dgraham commented Feb 22, 2017

The native Headers implementation in Chrome, Firefox, and Edge accepts an array in the constructor. IE 11 and Safari 10, where this polyfill is active, does not, so this is indeed a bug in the Headers polyfill.

Thanks for the detailed bug report and fix.

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! This looks good to me. @dgraham any reason why you didn't merge yet?

} else if (Array.isArray(headers)) {
headers.forEach(function(header) {
this.append(header[0], header[1]);
}, this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use semicolons.

@dgraham dgraham merged commit 9630aaa into JakeChampion:master Feb 22, 2017
@tschaub tschaub deleted the headers-array branch February 22, 2017 15:36
@tschaub
Copy link
Contributor Author

tschaub commented Mar 1, 2017

Thanks for pulling this in @dgraham. Any chance for an updated release with the fix?

@dgraham
Copy link
Contributor

dgraham commented Mar 2, 2017

The 2.0.3 release is now available on npm.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants