Use Web IDL's new record type #412
Conversation
There was a problem hiding this comment.
Overall this looks great. Thank you for working on this. I have a couple nits and we need some tooling changes that I requested here: plinss/widlparser#18.
I'd also appreciate it if you could add your name to the Acknowledgments section, but no worries if you don't want to.
|
|
||
| <p>See <a href=https://github.com/whatwg/fetch/issues/164>issue #164</a> for discussion. | ||
| </div> | ||
| <pre data-no-idl class=idl>typedef (Headers or sequence<sequence<ByteString>> or record<ByteString, ByteString>) <dfn id=headersinit typedef export>HeadersInit</dfn>;</pre> |
There was a problem hiding this comment.
The "data-no-idl" can be removed now this is all part of IDL.
There was a problem hiding this comment.
That and the manual <dfn> removed.
|
|
||
| <pre> | ||
| var meta = { "Content-Type": "text/xml", "Breaking-Bad": "<3" } | ||
| new Headers(meta)</pre> |
There was a problem hiding this comment.
I would prefer to keep this example. Change the class on the <div> to example and change the opening paragraph to something like: Constructing a {{Headers}} object is fairly straightforward:
There was a problem hiding this comment.
Kept and added to it example of creating an object with the sequence syntax.
| <li><p><a lt=append for=Headers>Append</a> | ||
| <var>header</var>'s key/<var>header</var>'s value to | ||
| <var>key</var>/<var>value</var> to | ||
| <var>headers</var>. Rethrow any exception. |
There was a problem hiding this comment.
Since this is now the only substep you should just inline it in the paragraph that introduces it. "then for each mapping ... in object, append ..."
|
|
||
| <ol> | ||
| <li><p>Set <var>header</var>'s key to <var>header</var>'s key, | ||
| <a spec=webidl>converted to ByteString</a>. |
There was a problem hiding this comment.
Since this is removed we can remove the corresponding line in class=anchors.
There was a problem hiding this comment.
Fixed, but I added a reference to the definition of a record mapping.
|
Thanks for fixing the IDL parser issue. Next up is speced/bikeshed#864 and then we can move on here. 😊 |
|
Found one more issue that needs figuring out: whatwg/webidl#233. Sorry about that. This one has quite a few dependencies. 😊 |
| @@ -24,7 +24,7 @@ Translate IDs: typedefdef-bodyinit bodyinit,typedefdef-responsebodyinit response | |||
|
|
|||
| <pre class=anchors> | |||
| url:https://xhr.spec.whatwg.org/#concept-formdata-entry;spec:XHR;type:dfn;text:entries | |||
| url:https://heycam.github.io/webidl/#es-ByteString;type:dfn;text:converted to ByteString;spec:WEBIDL | |||
| url:https://heycam.github.io/webidl/#record-mappings;type:dfn;text:mapping;spec:WEBIDL | |||
There was a problem hiding this comment.
These are not picked-up by Shepherd yet?
There was a problem hiding this comment.
"mappings" should soon be picked up per the change to IDL today. My plan was on updating this PR after that.
|
I forgot that I wanted to link record too and that IDL does not export that yet. So I guess this needs to wait yet another day. |
|
Sorry about that. I did push my final nits. Would appreciate if you could look those over. |
|
Thanks @TimothyGu, finally all the dependencies are resolved and this could land. |
|
👍 |
This is my first PR here, so please point out if I'm doing anything wrong.
Closes #164.