add 'JSON stringify and UTF-8 encode to bytes' algorithm #207
Conversation
There was a problem hiding this comment.
Mostly LGTM, with minor suggestions and the missing link thing. Will mark "do not merge yet" until the corresponding ECMAScript change is ready.
As for participant agreement, can you join the "paypal2" GitHub organization, or make public your membership? https://help.github.com/articles/publicizing-or-hiding-organization-membership/
infra.bs
Outdated
| @@ -1042,6 +1042,19 @@ as 200/`<code>OK</code>`. | |||
| <p class=note>The conventions used in this step are those of the JavaScript specification. | |||
| </ol> | |||
|
|
|||
| <p>To <dfn export>JSON stringify and UTF-8 encode to bytes</dfn> a given JavaScript value | |||
There was a problem hiding this comment.
Maybe omit "and UTF-8 encode" for symmetry? Since we assume everything is UTF-8 these days? Or maybe we should keep it.
There was a problem hiding this comment.
I personally would prefer it to be explicit. but y'all are editors of this spec, so up to you. IIRC that is the phrase that Anne suggested a while back.
infra.bs
Outdated
| <p>Let <var>jsonString</var> be the result of | ||
| <a abstract-op>Call</a>(<a>%JSONStringify%</a>, undefined, « <var>value</var> »). | ||
| [[!ECMA-262]] | ||
| <p class=note>The conventions used in this step are those of the JavaScript specification. |
There was a problem hiding this comment.
Maybe add <p class=note>Since no additional arguments are passed to <a>%JSONStringify%</a>, the resulting string will have no whitespace inserted</p>
There was a problem hiding this comment.
I added your suggested Note to the existing Note. if not OK will make it a separate note, see a3777de .
infra.bs
Outdated
| <ol> | ||
| <li> | ||
| <p>Let <var>jsonString</var> be the result of | ||
| <a abstract-op>Call</a>(<a>%JSONStringify%</a>, undefined, « <var>value</var> »). |
There was a problem hiding this comment.
I think you'll need to add a link thingy to the top of the document for JSONStringify, like was done for JSONParse.
There was a problem hiding this comment.
good catch, thx.
|
my paypal2 org membership is already public AFAICT: ..? |
|
@domenic |
|
Heh, I imagine you can see that, as you're a member. But if you use incognito mode or similar, you'll find that PayPal2 currently has no public members: So, you need to follow the above link's instructions and publicize. |
doh, sorry. I think it is fixed now. |
There was a problem hiding this comment.
LGTM. @annevk mind checking? Would like your thoughts on the naming as well. (At first I was going to suggest "JSON stringify to bytes", but then I realized that makes no sense. Maybe just "serialize JSON to bytes"?)
Unclear whether we should wait for tc39/ecma262#1272 or not. I'll poke it.
|
We call the other "parse JSON from bytes". It would make sense then to start this one with "stringify". But I guess "stringify JSON to bytes" is a little weird. I don't have a strong opinion here. It does seem that the downstream PR landed already, so if we make a change we'd have to change that too. |
|
@annevk "stringify" and "to bytes" don't mentally go together to me, since to me the starting state is the string, while the bytes are just data (payload). How about "serialize"? |
|
Yeah that works, it's just not what JavaScript^WECMAScript calls it. |
|
I'm a fan of serialize. I'll update this, merge, and also submit a PR to webauth as recompense for us being wishy washy over here :) |
|
thanks guys :) |
This is a follow-up to w3c#1017 to use the final name arrived at in whatwg/infra#207 and merged into the Infra Standard.
This is a follow-up to #1017 to use the final name arrived at in whatwg/infra#207 and merged into the Infra Standard.
this fixes #193.
note that this attendant change to ECMA262 needs to also be accomplished:
https://mail.mozilla.org/pipermail/es-discuss/2018-March/050324.html
see also: w3c/webauthn#712
HTH,
=JeffH
ps: we PayPal signed the whatwg participant agreement, see: https://github.com/whatwg/participant-data/blob/master/entities.json
Preview | Diff