Skip to content

Add Headers to HeadersInit - #321

Merged
Mohamed Hegazy (mhegazy) merged 1 commit into
masterfrom
addHeadersToHeadersInit
Nov 6, 2017
Merged

Mohamed Hegazy (mhegazy) merged 1 commit into
masterfrom
addHeadersToHeadersInit

Conversation

@mhegazy

Copy link
Copy Markdown
Contributor

Missed this in #315

@timocov

Copy link
Copy Markdown
Contributor

It is not missed - I have did it specially because the spec does not have it :-)

@timocov

Copy link
Copy Markdown
Contributor

For me this changes is a bit like change string[][] to [string, string][] - it is more correct and it will works but it does not correspond the spec.

@timocov

Copy link
Copy Markdown
Contributor

Mohamed Hegazy (@mhegazy) so why this changes should be but [string, string][] not? :-)

@mhegazy

Copy link
Copy Markdown
Contributor Author

record<ByteString, ByteString> is not really { [x:string]: string }. the later is much more restrictive than the earlier.

Header would be a valid record<ByteString, ByteString> but not a valid { [x:string]: string }. this was partially the behavior before the change by using object.

I did not want to go back to object, so just left it at Header which is a valid use case of this API, see https://developer.mozilla.org/en-US/docs/Web/API/Headers/Headers

@saschanaz

Copy link
Copy Markdown
Contributor

the later is much more restrictive than the earlier.

Indeed, new Headers(new Map([['abc', 'bcd']])).get('abc') returns bcd on Firefox and Chrome.

@saschanaz

Kagami Sascha Rosylight (saschanaz) commented Nov 7, 2017 •

Copy link
Copy Markdown
Contributor

new Headers({ [Symbol.iterator]() { return new Map([['abc', 'bcd']]).entries() } }).get('abc') returns bcd so I think we can check iterator type rather than adding Headers?

PS: Filed an issue for Edge returning null.

@saschanaz

Kagami Sascha Rosylight (saschanaz) commented Nov 7, 2017 •

Copy link
Copy Markdown
Contributor
type Record<K, V> = { [Symbol.iterator](): IterableIterator<[K, V]> } | { [x: K]: V };

This should also work for URLSearchParams receiving records.

new URLSearchParams({ [Symbol.iterator]() { return new Map([['abc', 'bcd']]).entries() } }).toString()
// returns 'abc=bcd'

@mhegazy

Copy link
Copy Markdown
Contributor Author

Headers is not iterable in --target ES5.. not sure what is the best way here. we can go back to object..

@saschanaz

Copy link
Copy Markdown
Contributor

Filed microsoft/TypeScript#19806.

@timocov

Copy link
Copy Markdown
Contributor

which is a valid use case of this API, see https://developer.mozilla.org/en-US/docs/Web/API/Headers/Headers

But it is not spec :)

Nobody known the truth: MDN has no case with string[][] as Headers ctor params, whatwg - Headers as Headers ctor params (and it is very strange) - I guess we need to use intersection of these specs and pass only { [key: string]: string } as valid Headers ctor params :)

@mhegazy

Copy link
Copy Markdown
Contributor Author

I guess we need to use intersection of these specs and pass only { [key: string]: string } as valid Headers ctor params :)

that is not correct.. A Header is not { [key:string]: string }, and so is a Map<string, string>. which are all valid use cases of this API.

@timocov

Copy link
Copy Markdown
Contributor

record<ByteString, ByteString> is not really { [x:string]: string }. the later is much more restrictive than the earlier.

Mohamed Hegazy (@mhegazy) could you please provide some examples to understand?

A Header is not { [key:string]: string }

Under as valid Headers ctor params I mean HeadersInit not Headers

@mhegazy

Copy link
Copy Markdown
Contributor Author

Here is an example : https://github.com/firebase/firebase-js-sdk/blob/0a6ad04befd820ec52b05040e86ff52a62e37fa5/packages/messaging/src/models/token-manager.ts#L173-L198

Header should be a valid header value. but Header is not assignable to { [x:string]: string }.

@timocov

Evgeniy Timokhov (timocov) commented Nov 7, 2017 •

Copy link
Copy Markdown
Contributor

Mohamed Hegazy (@mhegazy) Sorry, but I still cannot recognize the difference.

The spec says that Headers might be constructed from HeadersInit value.
HeadersInit in its turn should be sequence<sequence<ByteString>> or record<ByteString, ByteString>.
sequence<T> is lists of values of type T.
record<K, V> is ordered map with keys that are instances of K and values that are instances of V.

According to this I have a few questions:

  1. Why you say that record<ByteString, ByteString> is not really { [x:string]: string }? Of course it should be { [key: ByteString]: ByteString } (or even Map<ByteString, ByteString> also) but seems that it is not important for now.

Header would be a valid record<ByteString, ByteString> but not a valid { [x:string]: string }.

What do you mean under Header: Headers or HeadersInit or something else (according the spec)?

  1. What is valid header value?

Please correct me if I am somewhere wrong.

@mhegazy

Copy link
Copy Markdown
Contributor Author

record<K,V> talks about own properties.. does not look at methods.. {[x:string]: string} applies to all members of the type, methods or properties.. that is why Header is not {[x:string]: string} in TS but is record<string, string> in the spec

@timocov

Copy link
Copy Markdown
Contributor

Oh, I get it, thank you!

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