w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
326 stars 55 forks source link

Response.json() #741

Closed ricea closed 2 years ago

ricea commented 2 years ago

Braw mornin' TAG!

I'm requesting a TAG review of Response.json().

Response.json(body) is a static method used to construct a Response object from the JSON serialisation of "body". It does what you would want new Response(body) to do if that was not ambiguous.

Further details:

You should also know that...

It is a very small feature.

We'd prefer the TAG provide feedback as (please delete all but the desired option):

💬 leave review feedback as a comment in this issue and @-notify ricea, annevk, lucacasonato

¹ We require an explainer to give the relevant context for the spec review, even if the spec has some background information. For background, see our explanation of how to write a good explainer. We recommend the explainer to be in Markdown.

² A Security and Privacy questionnaire helps us understand potential security and privacy issues and mitigations for your design, and can save us asking redundant questions. See https://www.w3.org/TR/security-privacy-questionnaire/.

torgo commented 2 years ago

Any chance you could publish the explainer and questionnaire responses in markdown along side of the spec please?

ricea commented 2 years ago

I've created Markdown versions at https://github.com/ricea/response-json-explainer.

@annevk how would you feel about including them in the fetch repository?

annevk commented 2 years ago

I could see adding an example to the standard. WHATWG standards are expected to be self-contained documents so I wouldn't really want to add separate markdown files that also need to be consulted/maintained.

LeaVerou commented 2 years ago

Hi @ricea,

We took a look at this today during a breakout. While the functionality seems worthwhile, we are concerned that it might be confusing that there is an instance method and a static method with the same name that do exactly opposite things. This is also acknowledged in the explainer, but as a benefit:

This is symmetric with the Response.prototype.json() method which converts back the other way.

We are also unsure if it's a good idea to have a factory method whose naming does not indicate that it's a factory method.

What are your thoughts on this? What alternative names have you considered?

LeaVerou commented 2 years ago

Hi again. We looked at this again during a breakout today.

We still think that it's confusing if any static method could be a factory method, and we believe there is value in making this obvious with a naming convention. As an example, we explored ideas such as Response.from(value, options) with a dictionary argument whose type defaults to "json", which would be equally concise.

However, we see that there is precedent in having factory methods in this API whose names are simple nouns (Response.error(), Response.redirect()), and we think internal consistency within the API is more important than external consistency with the rest of the Web Platform, so we will go ahead and close this.

ricea commented 2 years ago

Thank you for the feedback. I like the idea of Response.from() but since most types can simply be passed to the constructor, it might turn out to be a misleadingly general name.