Skip to content
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

Improve documentation for no-cors mode #36476

Merged
merged 2 commits into from
Oct 25, 2024
Merged

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Oct 24, 2024

Fixes #35488.

The immediate thing here is that you can't set Range when making a no-cors request, even though Range is a CORS-safelisted request header. So everywhere that we said "with no-cors, you can only set CORS-safelisted request headers" needed to be updated.

I didn't want to duplicate these details too much and wasn't sure where to put them. There are three pages I could see that needed updating:

I ended up putting most of the details in RequestInit, because that's the point where you set mode, and because you're more likely to get to that page from (especially) the service worker documentation. Request.mode is just a read-only reflection of the value set in RequestInit.

I also tried to update no-cors to explain what it actually does, what the implications of using it are (especially, what an opaque response is), and when you might want to use it. It seems like a lot of people are confused about it, thinking it's a way to get around CORS errors, which it really isn't.

Sources:

@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Oct 24, 2024
Copy link
Contributor

github-actions bot commented Oct 24, 2024

Preview URLs

(comment last updated: 2024-10-24 17:23:58)

@wbamberg wbamberg marked this pull request as ready for review October 24, 2024 17:36
@wbamberg wbamberg requested a review from a team as a code owner October 24, 2024 17:36
@wbamberg wbamberg requested review from sideshowbarker and removed request for a team October 24, 2024 17:36
Copy link
Member

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

👍 Really nice additions/changes

@sideshowbarker sideshowbarker merged commit 0129176 into mdn:main Oct 25, 2024
8 checks passed
@wbamberg
Copy link
Collaborator Author

👍 Really nice additions/changes

<3 I copied a lot of it from your SO answers, so thank you for those!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request's mode incorrectly documented
2 participants