-
Notifications
You must be signed in to change notification settings - Fork 349
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Editorial: Add prose about constructing a request #1585
Merged
Merged
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
1963b40
Editorial: Add prose about constructing a request
noamr e409fd1
Rephrase to be more howto style
noamr 8e2b505
typo
noamr b21fe3d
Update fetch.bs
noamr 690004d
Update fetch.bs
noamr ba70f23
Update fetch.bs
noamr 7debfee
Update fetch.bs
noamr 13c12bc
Update fetch.bs
noamr 1ade14c
Update fetch.bs
noamr 8330d90
Update fetch.bs
noamr 0f756f4
Update fetch.bs
noamr 6657a94
Update fetch.bs
noamr bd6e06e
Update fetch.bs
noamr 68a719a
Add section about no-client requests
noamr d1cfa86
Update fetch.bs
noamr af0db38
Address some notes
noamr 105475b
Add redirect-mode
noamr 6152d2a
nits
noamr f028f4b
Update fetch.bs
noamr baf471f
Update fetch.bs
noamr 03ad3e6
Update fetch.bs
noamr 389f0f2
Update fetch.bs
noamr 696b455
Remove unnecessary table changes
noamr db14cac
Update fetch.bs
noamr dd89623
nits
annevk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Update fetch.bs
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
- Loading branch information
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might just have more contact with unusual APIs than most people, but I feel like "there's no obvious client" comes up fairly often. Could we say "If your request isn't associated with a particular environment settings object, please start a discussion with this repository's maintainers?" Ideally, we'd be able to link to a forum on which to start that discussion...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give examples outside of FedCM? Is this mostly about UA-initiated fetches that are not web-exposed in themselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe these are all web-exposed:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the fetch itself is not web exposed?
This is a JS API with an IDL interface, which has a relevant settings object.
Seems like this ends up also doing something with a JS observer, which has a relevant settings object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the first, I might be understanding something different by "web-exposed": the result doesn't get reported to Javascript running in a web page, but developers building websites will have their designs broken if it changes.
For the second, the relevant settings object's window may have closed by the time the fetch is sent, which means it can't use that as its client. Maybe the right answer is to start a Service Worker to exist while the fetch is running, so that can be the client, but the feature's designers need someone to tell them that, and help work through the tradeoffs.
For the third, the JS observer isn't really important, but there is a global available in the algorithm's caller, so the use of
null
is just a bug. But the developers thought they didn't have one, maybe because the algorithm originally ran after the originating client closed. They should have a way to ask what to do, so someone knowledgeable can find them the right client.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree that documentation for this is lacking. Let's fix it step by step.
btw client is not mandatory. Instead of a client you can populate the request's origin and policy container, and respond to callbacks on a parallel queue (some anonymous new thread). For some of these features maybe that's the right approach and we should address this in this documentation.