Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

AddResponse.json static method#1392

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
annevk merged 16 commits intowhatwg:mainfromlucacasonato:response_json_method
May 18, 2022

Conversation

lucacasonato
Copy link
Member

@lucacasonatolucacasonato commentedJan 30, 2022•
edited by pr-previewbot
Loading

This commit adds aResponse.json static method that can be used to
create well formed JSON responses will very little effort.

The JSON response isnot pretty printed.

Closes#1389

Preview |Diff

This commit adds a `Response.json` static method that can be used tocreate well formed JSON responses will very little effort.
@domenic
Copy link
Member

Note that we'd need to fix Web IDL first to allow regular and static operations with the same name:

The identifier of a static operation also must not be the same as the identifier of a regular operation defined on the same interface.

fromhttps://webidl.spec.whatwg.org/#idl-operations

@lucacasonato
Copy link
MemberAuthor

@domenic Do you think that is desirable to change? It may be confusing to have both a prototype and static method with the same name. Maybe this method should be calledfromJSON.

rauschma reacted with thumbs up emoji

@domenic
Copy link
Member

I think it is desirable to change and do not see anything confusing about having such methods, personally.

annevk and lucacasonato reacted with thumbs up emoji

Copy link
Member

@annevkannevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This looks reasonable, but I think it can be improved by making initialize a response also take a body and Content-Type header value (potentially as a tuple?). That would allow for sharing even more steps.

I also wonder what we should do here if browser interest isn't forthcoming. Perhaps there's a way in which we can still have mutual agreement, but it's not exposed in browsers for now. Perhaps this is worth bringing up in the HTML triage call (if it doesn't otherwise get resolved).

@domenicdomenic mentioned this pull requestFeb 14, 2022
3 tasks
@jasnell
Copy link

Just fyi... I have an implementation of this in Cloudflare Workers just waiting on this to land or at least signal that it's moving forward.

@domenic
Copy link
Member

domenic commentedMar 8, 2022•
edited
Loading

I don't think we have any signal that this is moving forward in browsers, unfortunately, due to it just being low-priority sugar for browser teams. As such I don't think it'll land in Fetch, at least until a couple of browsers have engineers looking for starter projects.

I'd really encourage you to use this as a case study forhttps://github.com/whatwg/js-hosts . Figure out what sort of documentation you want to create there, which would enable you to land such things with confidence, and go for it! I'm happy to participate and secure guarantees like "Chromium will never ship aResponse.json() with different semantics than those specified here".

@lucacasonato
Copy link
MemberAuthor

@domenic Would this get a 👍 from Chromium if we did the implementation work?

@domenic
Copy link
Member

@ricea would be the decider there but I can predict with very high probability he would support it :).

lucacasonato and jasnell reacted with thumbs up emoji

@annevkannevk added the needs implementer interestMoving the issue forward requires implementers to express interest labelMar 10, 2022
Copy link
Member

@annevkannevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Mostly nits and request for a follow-up. Shared algorithm looks rather good! Thanks.

Copy link
Member

@annevkannevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah I know, it's just sad and mostly a consequence of some early ideas aroundtext/* that ended up not mattering (at least not on the web).

Anyway, thanks for all your hard work on this. This was not an easy feature given all the work on Web IDL, refactoring of various Fetch algorithms, etc. 😊

I suggest we give folks until May 18 to voice any remaining concerns and merge it that day sometime.

lucacasonato reacted with thumbs up emojilucacasonato reacted with hooray emoji
annevk pushed a commit to web-platform-tests/wpt that referenced this pull requestMay 18, 2022
@annevkannevk added addition/proposalNew features or enhancements topic: api labelsMay 18, 2022
@annevkannevk merged commitb3bfd0c intowhatwg:mainMay 18, 2022
@foolip
Copy link
Member

This change isn't possible to roll into WPT's idlharness.js test, seeweb-platform-tests/wpt#34155. Does anyone want to give it a go to update idlharness.js to avoid this problem?

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull requestMay 24, 2022
…stonlyAutomatic update from web-platform-testsFetch: add tests for Response.jsonContext:whatwg/fetch#1392.--wpt-commits: 27e770bcfbeae534b5d4cbc0a50d2b8247942312wpt-pr: 32825
jamienicol pushed a commit to jamienicol/gecko that referenced this pull requestMay 25, 2022
…stonlyAutomatic update from web-platform-testsFetch: add tests for Response.jsonContext:whatwg/fetch#1392.--wpt-commits: 27e770bcfbeae534b5d4cbc0a50d2b8247942312wpt-pr: 32825
aarongable pushed a commit to chromium/chromium that referenced this pull requestJun 30, 2022
This commit adds support for the `Response.json` static method asspecified inwhatwg/fetch#1392.All WPTs fromweb-platform-tests/wpt#32825 pass.Bug:1305358Change-Id: Iaafdd514ed12644b6433c02e7d076ce43f51b9efReviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/3639064Reviewed-by: Yutaka Hirano <yhirano@chromium.org>Reviewed-by: Adam Rice <ricea@chromium.org>Commit-Queue: Yutaka Hirano <yhirano@chromium.org>Cr-Commit-Position: refs/heads/main@{#1019479}
webkit-commit-queue pushed a commit to andreubotella/WebKit that referenced this pull requestMar 22, 2023
https://bugs.webkit.org/show_bug.cgi?id=240375Reviewed by Youenn Fablet.This implements the `Response.json` static method, added to the fetchspec inwhatwg/fetch#1392.* LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-static-json.any-expected.txt:* LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-static-json.any.serviceworker-expected.txt:* LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-static-json.any.sharedworker-expected.txt:* LayoutTests/imported/w3c/web-platform-tests/fetch/api/response/response-static-json.any.worker-expected.txt:* Source/WebCore/Modules/fetch/FetchBody.h:* Source/WebCore/Modules/fetch/FetchResponse.cpp:(WebCore::FetchResponse::create):  Added this method overload to implement the spec's "initialize a  response" algorithm. This algortihm used to be combined with the  regular Response creation algorithm which extracts the body, but  Response.json cannot use that directly.(WebCore::FetchResponse::staticJson):* Source/WebCore/Modules/fetch/FetchResponse.h:* Source/WebCore/Modules/fetch/FetchResponse.idl:Canonical link:https://commits.webkit.org/261960@main
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@yutakahiranoyutakahiranoyutakahirano left review comments

@andreubotellaandreubotellaandreubotella left review comments

@annevkannevkannevk approved these changes

Assignees
No one assigned
Labels
addition/proposalNew features or enhancementstopic: api
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Proposal:Response.json helper
8 participants
@lucacasonato@domenic@jasnell@ricea@yutakahirano@annevk@foolip@andreubotella
[8]ページ先頭
©2009-2025 Movatter.jp