Skip to content

Doc cors spec compliance#4202

Merged
mcollina merged 4 commits intonodejs:mainfrom
FelixVaughan:doc-cors-spec-compliance
May 10, 2025
Merged

Doc cors spec compliance#4202
mcollina merged 4 commits intonodejs:mainfrom
FelixVaughan:doc-cors-spec-compliance

Conversation

@FelixVaughan
Copy link
Contributor

This PR adds a section to the documentation detailing the default CORS behaviour mentioned in #4179.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Lgtm

##### Garbage Collection
#### CORS

Unlike browsers, Undici does not implement CORS (Cross-Origin Resource Sharing) checks by default. This means:
Copy link
Member

Choose a reason for hiding this comment

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

Adding a reference to the relevant part of the fetch spec would be useful.

Copy link
Contributor Author

@FelixVaughan FelixVaughan May 12, 2025

Choose a reason for hiding this comment

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

Would a link to this section suffice? If so I can address in a separate PR.
https://fetch.spec.whatwg.org/#cors-check

##### Garbage Collection
#### CORS

Unlike browsers, Undici does not implement CORS (Cross-Origin Resource Sharing) checks by default. This means:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Unlike browsers, Undici does not implement CORS (Cross-Origin Resource Sharing) checks by default. This means:
Unlike browsers, Undici does not implement CORS (Cross-Origin Resource Sharing) checks. This means:

"by default" makes it sound like there is a way to enable CORS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Will address if a new PR is created for the suggestion above.

@mcollina mcollina merged commit 0981779 into nodejs:main May 10, 2025
42 of 47 checks passed
This was referenced May 10, 2025
caitp pushed a commit to caitp/undici that referenced this pull request May 15, 2025
slagiewka pushed a commit to slagiewka/undici that referenced this pull request Feb 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants