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

Change Headers multiple value handling for spec compatibility #429

Merged
merged 2 commits into from Nov 14, 2016
Merged

Change Headers multiple value handling for spec compatibility #429

merged 2 commits into from Nov 14, 2016

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Nov 10, 2016

Consider this Headers object:

var h = new Headers()
h.append('accept', 'text/html')
h.append('accept', 'text/plain')
h.append('content-type', 'application/json')

Before:

  • h.get('accept') returned text/html
  • h.getAll('accept') returned an array of values
  • h.forEach (and other iterables) did distinct iterations for each value of the same header

Now:

  • h.get('accept') returns text/html,text/plain
  • h.getAll() is no more
  • h.forEach (and other iterables) now only do one iteration for each headers name, regardless of multiple values

This is in accordance with Section 3.1.2 of the spec, "combine" concept. whatwg/fetch@42464c8

The updated tests currently break in Chrome and Firefox when exercising the native implementation because their implementation is outdated. The implementation in Edge is more correct, but the tests still fail there because its implementation combines values with , (notice the space) rather than ,.

Consider this Headers object:

    var h = new Headers()
    h.append('accept', 'text/html')
    h.append('accept', 'text/plain')
    h.append('content-type', 'application/json')

Before:

- `h.get('accept')` returned `text/html`
- `h.getAll('accept')` returned an array of values
- `h.forEach` (and other iterables) did distinct iterations for each
  value of the same header

Now:

- `h.get('accept')` returns `text/html,text/plain`
- `h.getAll()` is no more
- `h.forEach` (and other iterables) now only do one iteration for each
  headers name, regardless of multiple values

This is in accordance with Section 3.1.2 of the spec, "combine" concept.

The updated tests currently break in Chrome and Firefox when exercising
the native implementation because their implementation is outdated. The
implementation in Edge is more correct, but the tests still fail there
because its implementation combines values with `, ` (notice the space)
rather than `,`.
@mislav mislav merged commit fc226e8 into JakeChampion:master Nov 14, 2016
@mislav mislav deleted the headers-spec-compat branch November 14, 2016 19:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 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.

None yet

1 participant